Skip to content

Fixes retrieval of the latest snapshot index blob#22700

Merged
abeyad merged 2 commits intoelastic:masterfrom
abeyad:index_latest_retrieval
Jan 20, 2017
Merged

Fixes retrieval of the latest snapshot index blob#22700
abeyad merged 2 commits intoelastic:masterfrom
abeyad:index_latest_retrieval

Conversation

@abeyad
Copy link
Copy Markdown

@abeyad abeyad commented Jan 19, 2017

This commit ensures that the index.latest blob is first examined to
determine the latest index-N blob id, before attempting to list all
index-N blobs and picking the blob with the highest N.

It also fixes the MockRepository#move so that tests are able to handle
non-atomic moves. This is done by adding a special setting to the
MockRepository that requires the test to specify if it can handle
non-atomic moves. If so, then the MockRepository#move operation will be
non-atomic to allow testing for against such repositories.

This commit ensures that the index.latest blob is first examined to
determine the latest index-N blob id, before attempting to list all
index-N blobs and picking the blob with the highest N.

It also fixes the MockRepository#move so that tests are able to handle
non-atomic moves.  This is done by adding a special setting to the
MockRepository that requires the test to specify if it can handle
non-atomic moves.  If so, then the MockRepository#move operation will be
non-atomic to allow testing for against such repositories.
Copy link
Copy Markdown
Contributor

@imotov imotov left a comment

Choose a reason for hiding this comment

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

Left a couple of minor comments.

@@ -733,22 +733,21 @@ void writeIncompatibleSnapshots(RepositoryData repositoryData) throws IOExceptio
long latestIndexBlobId() throws IOException {
try {
// first, try listing the blobs and determining which index blob is the latest
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This comment is probably wrong now.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

done

blockOnInitialization = metadata.settings().getAsBoolean("block_on_init", false);
randomPrefix = metadata.settings().get("random", "default");
waitAfterUnblock = metadata.settings().getAsLong("wait_after_unblock", 0L);
nonAtomicMove = metadata.settings().getAsBoolean("non_atomic_move", false);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think negatively named settings are somewhat difficult to reason about. For example, I think it's much easier to understand that atomicMove = true is than nonAtomicMove = false.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

i changed it to atomic_move

Settings.builder()
.put("location", repoPath)
.put("random_control_io_exception_rate", randomIntBetween(5, 20) / 100f)
.put("non_atomic_move", true)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not really sure why this test is singled out. Maybe add comment?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

done

@abeyad abeyad added v5.2.1 and removed v5.2.0 labels Jan 20, 2017
@abeyad abeyad merged commit 3bf06d1 into elastic:master Jan 20, 2017
@abeyad abeyad deleted the index_latest_retrieval branch January 20, 2017 23:00
abeyad pushed a commit that referenced this pull request Jan 20, 2017
This commit ensures that the index.latest blob is first examined to
determine the latest index-N blob id, before attempting to list all
index-N blobs and picking the blob with the highest N.

It also fixes the MockRepository#move so that tests are able to handle
non-atomic moves.  This is done by adding a special setting to the
MockRepository that requires the test to specify if it can handle
non-atomic moves.  If so, then the MockRepository#move operation will be
non-atomic to allow testing for against such repositories.
abeyad pushed a commit that referenced this pull request Jan 20, 2017
This commit ensures that the index.latest blob is first examined to
determine the latest index-N blob id, before attempting to list all
index-N blobs and picking the blob with the highest N.

It also fixes the MockRepository#move so that tests are able to handle
non-atomic moves.  This is done by adding a special setting to the
MockRepository that requires the test to specify if it can handle
non-atomic moves.  If so, then the MockRepository#move operation will be
non-atomic to allow testing for against such repositories.
@abeyad abeyad removed the v5.1.3 label Jan 20, 2017
@abeyad
Copy link
Copy Markdown
Author

abeyad commented Jan 20, 2017

5.x: ff0350d
5.2: 35f5729

jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jan 23, 2017
* master: (33 commits)
  Docs fix - Added missing link to new Adjacency-matrix agg
  Pass `forceExecution` flag to transport interceptor (elastic#22739)
  Version: Add missing releases from 2.x in Version.java (elastic#22594)
  CONSOLE-ify filter aggregation docs
  CONSOLE-ify date_range aggregation docs
  Add single static instance of SpecialPermission (elastic#22726)
  Simplify InternalEngine#innerIndex (elastic#22721)
  Upgrade to Lucene 6.4.0 (elastic#22724)
  Fix broken TaskInfo.toString()
  Add CheckedSupplier and CheckedRunnable to core (elastic#22725)
  Revert "Make build Gradle 2.14 / 3.x compatible (elastic#22669)"
  Fixes retrieval of the latest snapshot index blob (elastic#22700)
  CONSOLE-ify date histogram docs
  CONSOLE-ify min and max aggregation docs
  CONSOLE-ify global-aggregation.asciidoc
  Fix script score function that combines _score and weight (elastic#22713)
  Corrected a plural verb to a singular one. (elastic#22681)
  Fix duplicates from search.query (elastic#22701)
  Readd unconverted snippets mark for doc
  Deguice rest handlers (elastic#22575)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >enhancement v5.2.0 v6.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants