yiisoft / yii2-elasticsearch

Yii 2 Elasticsearch extension
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
427 stars 253 forks source link

Executes are repeatedly executed in the loop body. Actions are not re… #307

Closed jinpy666 closed 3 years ago

jinpy666 commented 3 years ago

…leased, resulting in duplicate actions. Memory growth causes the program to crash.

Q A
Is bugfix? yes
New feature? yes/no
Breaks BC? yes/no
Tests pass? yes
Fixed issues comma-separated list of tickets # fixed by the PR, if any

image

bizley commented 3 years ago

How do you initialize $commander?

jinpy666 commented 3 years ago

How do you initialize $commander?

I didn't initialize it every time in the loop.

image

bizley commented 3 years ago

I think that you should either use it the way you did or call createBulkCommand() inside the loop.

jinpy666 commented 3 years ago

I think that you should either use it the way you did or call createBulkCommand() inside the loop.

It works because I read through the source code. If someone else doesn't know about it and the documentation isn't perfect, I'll run into this inexplicable problem. I think adding this Actions release will make the code more robust😄

bizley commented 3 years ago

That's a good point but since the change would make it behave different way than before and thus it would be BC break, let's hear what others have to say.

samdark commented 3 years ago

@beowulfenator what do you think?

beowulfenator commented 3 years ago

I disagree.

There is no reason to remove anything inside the batch command after it has been executed. This would be as if a query self-destructed after running execute().

Besides, there may be legitimate use cases that depend on preserving the actions, for example, if you build the command once, but then run it several times on several different ES cluster. I know this is far-fetched, but still.

If you believe this behavior is not intuitive, feel free to add a note to the docs.