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

Add current translog ID to commit meta before closing #8245

Merged
merged 2 commits into from Oct 28, 2014

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Oct 27, 2014

Today we simply close the IW when we have to open a new one if ie.
we change merge settings. Yet, the API is deprecated and under the hood
it commits the IW. For safety we should add the current translog ID and
use rollback instead to not wait for merges.

@rjernst
Copy link
Member

rjernst commented Oct 27, 2014

LGTM.

currentIndexWriter().close(false);
{ // commit and close the current writer - we write the current tanslog ID just in case
final long translogId = translog.currentId();
indexWriter.setCommitData(MapBuilder.<String, String>newMapBuilder().put(Translog.TRANSLOG_ID_KEY, Long.toString(translogId)).map());
Copy link
Member

Choose a reason for hiding this comment

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

Asking for my own information here, but would it be more efficient to use new HashMap<String, String>(1) because we always know the capacity for the HashMap? The default constructor has an initialCapacity of 16, does it save much to manually lower it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

almost certainly - I just copy pasted this feel free to cut over the other places ;)

Today we simply close the IW when we have to open a new one if ie.
we change merge settings. Yet, the API is deprecated and under the hood
it commits the IW. For safety we should add the current translog ID and
use rollback instead to not wait for merges.
@s1monw
Copy link
Contributor Author

s1monw commented Oct 28, 2014

@dakrone I pushed another commit that fixes the map builder too

@dakrone
Copy link
Member

dakrone commented Oct 28, 2014

LGTM

@s1monw s1monw merged commit 31db8cc into elastic:master Oct 28, 2014
@s1monw s1monw deleted the remove_deprecated_close branch October 28, 2014 20:45
@s1monw s1monw removed the review label Oct 28, 2014
@clintongormley clintongormley changed the title [ENGINE] Add current translog ID to commit meta before closing Internal: Add current translog ID to commit meta before closing Nov 3, 2014
@clintongormley clintongormley changed the title Internal: Add current translog ID to commit meta before closing Add current translog ID to commit meta before closing 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
>bug :Distributed/Engine Anything around managing Lucene and the Translog in an open shard. v1.3.5 v1.4.0 v1.5.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants