Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

setRefresh #1475

Closed
qqqqb opened this issue Mar 20, 2018 · 2 comments
Closed

setRefresh #1475

qqqqb opened this issue Mar 20, 2018 · 2 comments

Comments

@qqqqb
Copy link

qqqqb commented Mar 20, 2018

I found you are accepting Bool only on refresh at lib/Elastica/AbstractUpdateAction.php #L328

While it should be

Empty string or true

Refresh the relevant primary and replica shards (not the whole index) immediately after the operation occurs, so that the updated document appears in search results immediately. This should ONLY be done after careful thought and verification that it does not lead to poor performance, both from an indexing and a search standpoint.

wait_for

Wait for the changes made by the request to be made visible by a refresh before replying. This doesn’t force an immediate refresh, rather, it waits for a refresh to happen. Elasticsearch automatically refreshes shards that have changed every index.refresh_interval which defaults to one second. That setting is dynamic. Calling the Refresh API or setting refresh to true on any of the APIs that support it will also cause a refresh, in turn causing already running requests with refresh=wait_for to return.

false (the default)

Take no refresh related actions. The changes made by this request will be made visible at some point after the request returns.
https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-refresh.html
@ruflin
Copy link
Owner

ruflin commented Mar 21, 2018

Definitively, we should support the above params. Do you want to open a PR with the change?

@qqqqb qqqqb mentioned this issue Mar 21, 2018
Closed
@knsarsaba knsarsaba mentioned this issue Aug 7, 2020
Merged
ruflin pushed a commit that referenced this issue Aug 10, 2020
@knsarsaba
Allow string argument in AbstractUpdateAction::setRefresh (#1791)
9146c80 
Ref #1475 

Currently, `AbstractUpdateAction::setRefresh` only accepts boolean values: 
- `true` - to perform an immediate refresh
- `false` - to not refresh at all

The [documentation](https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-refresh.html), however, strongly suggests the usage of `wait_for` option to ensure a good cluster performance while doing the refresh operation. But the problem is that `wait_for` is of type `string`; thus, we cannot pass it as an argument to the method.

This modifies `AbstractUpdateAction::setRefresh` so that arguments of type `string` such as `wait_for` can also be passed to the method. Additionally, the return type of its getter counterpart, `AbstractUpdateAction::getRefresh`, has also been modified to mirror the accepted types of the setter.
@thePanz
Copy link
Collaborator

thePanz commented Oct 27, 2021

Closing as #1791 got merged.

@thePanz thePanz closed this as completed Oct 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@thePanz @ruflin @qqqqb