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.

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

Remove dirty flag and force boolean for refresh #9484

Merged
merged 1 commit into from Jan 29, 2015

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Jan 29, 2015

Today we have a dirty flag indicating that a refresh must
be executed. We also allow users to bypass this by setting
a force=true boolean on the refresh request / command. All
these flags are unneeded since the SearcherManager has all
the information to do the right thing if it's dirty or not.

@@ -694,7 +684,7 @@ public boolean refreshNeeded() {
try {
// we are either dirty due to a document added or due to a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is a bit stale, e.g. says "dirty" (which is gone) and e.g. deletes can also mean we are not current ... I think just remove the comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@mikemccand
Copy link
Contributor

LGTM, what a nice cleanup: 2 booleans and 1 mutex removed!

@s1monw
Copy link
Contributor Author

s1monw commented Jan 29, 2015

@mikemccand I pushed updates

@mikemccand
Copy link
Contributor

LGTM, thanks @s1monw!

Today we have a dirty flag indicating that a refresh must
be executed. We also allow users to bypass this by setting
a force=true boolean on the refresh request / command. All
these flags are unneeded since the SearcherManager has all
the information to do the right thing if it's dirty or not.
@s1monw s1monw merged commit 03f1fcc into elastic:master Jan 29, 2015
indexShard.refresh("api", request.force());
logger.trace("{} refresh request executed, force: [{}]", indexShard.shardId(), request.force());
indexShard.refresh("api");
logger.trace("{} refresh request executed, force: [{}]", indexShard.shardId());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should remove the "force:[{}]" in trace logger. @s1monw

@mikemccand
Copy link
Contributor

Thanks @boliza I'll push a fix.

@s1monw s1monw deleted the remove_force_refresh branch January 30, 2015 23:04
@s1monw
Copy link
Contributor Author

s1monw commented Feb 4, 2015

@mikemccand @dakrone I think I should backport this to 1.x too. WDYT

@mikemccand
Copy link
Contributor

++ @s1monw

@dakrone
Copy link
Member

dakrone commented Feb 4, 2015

yes, +1 on backport to 1.x

@clintongormley clintongormley changed the title [ENGINE] Remove dirty flag and force boolean for refresh Remove dirty flag and force boolean for refresh Jun 7, 2015
@clintongormley clintongormley added :Distributed/Distributed A catch all label for anything in the Distributed Area. If you aren't sure, use this one. :Distributed/Engine Anything around managing Lucene and the Translog in an open shard. and removed :Engine :Distributed/Distributed A catch all label for anything in the Distributed Area. If you aren't sure, use this one. labels Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Engine Anything around managing Lucene and the Translog in an open shard. >enhancement v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants