Implement Eventually Consistent Mock Repository for SnapshotResiliencyTests #40893

original-brownbear · 2019-04-05T11:13:15Z

Implement simple simulation of an eventually consistent blob repository to be able to reproduce issues resulting from S3 Snapshot Repository Erroneously Assumes Consistent List Operation #38941. See new Javadoc added here for the exact behavior simulated.
- Added a todo to make the behavior include inconsistency over a longer period of time and specifically over multiple back to back snapshots and deletes that include node failures with concurrent indexing (currently we test this, but without changing the data in the indices snapshotted so we're not affected by the eventual consistent list operation on shard directories that would break things).
Fortunately, as a result of Name Snapshot Data Blobs by UUID #40652 we can now merge this in as the current tests are able to deal with S3 semantics since they don't rely on consistent list operations in what they test (they don't do any updates to a shard's segment files, so it all seems to work out fine (I ran 5k iterations of each test to confirm)).

* Reproduce S3 semantics exactly

…ent-mock-repo

elasticmachine · 2019-04-05T11:13:17Z

Pinging @elastic/es-distributed

andrershov · 2019-04-16T15:59:26Z

.../src/test/java/org/elasticsearch/snapshots/mockstore/MockEventuallyConsistentRepository.java

+
+            @Override
+            public InputStream readBlob(String name) throws IOException {
+                ensureReadAfterWrite(name);


I'm not sure, how we're simulating eventual consistency for non-first write operation here.
If there is pendingWrite, it seems that we're just executing it and perform read of the latest pendingWrite.

That's a fair point, thanks for spotting this! Practically it doesn't change anything (fortunately) since the index.latest blob is the only blob we ever overwrite (and we don't use that blob in normal blob repository operation) but I'll fix this shortly :)

…ent-mock-repo

original-brownbear · 2019-04-17T11:06:10Z

@andrershov fixed the incorrect consistency in e320ebc (I added a todo to make the simulation even less consistent, but I'm actually not sure that it's needed).

I also and (more importantly) fixed an issue with the extra0 files that Lucene leaves around that started breaking this test now that #40857 is merged. Technically that should/could be a separate PR but since it's an issue that's so much more pronounced for the async repository I put it in here since even without the extra0file we could run into empty trees since we have no guarantees that the delete for the directory runs after the deletes for the individual files.
This isn't a problem in a real (S3) repository because there isn't even any folders in S3 anyway.

…ent-mock-repo

original-brownbear · 2019-04-18T12:16:49Z

Lets get #41332 in first before we fix the remaining things here :)

andrershov

@original-brownbear If we truly want to emulate eventual consistency, we can just remember all writes done to some blob path as a list of byte[] in memory and each time read is executed, we can return random write.
So I would rather have EventualConsistentInMemoryBlobContainer implementing BlobContainer interface and use it in SnapshotResiliencyTests.
The implementation that you're proposing is hard to follow, contains bugs (as we figured out yesterday) and does not fully resemble AWS consistency model.
Also, I don't think it's a good idea to emulate only those AWS consistency phenomena that we currently support. I think we should implement proper eventual consistent blob container and plug it in, once we think all bugs (that assume stronger consistency) in our code are addressed.

…ent-mock-repo

ywelsch

I've asked for more tests

ywelsch · 2019-07-12T15:49:55Z

.../src/test/java/org/elasticsearch/snapshots/mockstore/MockEventuallyConsistentRepository.java

+                    if (writes.isEmpty()) {
+                        throw new NoSuchFileException(blobPath);
+                    }
+                    if (readBeforeWrite == false && writes.size() == 1) {


this does not take deletions into account.

This shows that the class here needs unit tests that validates its behavior.

ywelsch · 2019-07-12T15:54:24Z

.../src/test/java/org/elasticsearch/snapshots/mockstore/MockEventuallyConsistentRepository.java

+            @Override
+            public void delete() {
+                if (closed.get()) {
+                    throw new AssertionError("Blobstore is closed already");


perhaps factor this out into an ensureNotClosed() method

…-brownbear/elasticsearch into eventually-consistent-mock-repo

…ent-mock-repo

original-brownbear · 2019-07-16T06:07:37Z

@ywelsch I pushed d1f1e79

This adds tests and fixes the behavior on deletes. Also I removed the logic giving special case handling to incompatible-snapshots and index.latest since both aren't used by the snapshot logic (except for the URL repository but that's not relevant here) so there's no point having leniency for them now.

ywelsch

I've left some more comments.

.../src/test/java/org/elasticsearch/snapshots/mockstore/MockEventuallyConsistentRepository.java

ywelsch · 2019-07-16T08:24:24Z

.../src/test/java/org/elasticsearch/snapshots/mockstore/MockEventuallyConsistentRepository.java

+            public void deleteBlob(String blobName) {
+                ensureNotClosed();
+                synchronized (context.actions) {
+                    context.actions.add(new BlobStoreAction(Operation.DELETE, path.buildAsString() + blobName));


path.add(blobName).buildAsString()?

ywelsch · 2019-07-16T08:32:21Z

.../src/test/java/org/elasticsearch/snapshots/mockstore/MockEventuallyConsistentRepository.java

+            public void writeBlob(String blobName, InputStream inputStream, long blobSize, boolean failIfAlreadyExists)
+                throws IOException {
+                ensureNotClosed();
+                // TODO: Throw if we try to overwrite any blob other than incompatible_snapshots or index.latest with different content


what about this TODO? Can we enable this already for some files?

Yea we can for all but snap- (at the root level) and index-N at the root level I think. Will do :)

I way beefed this up now. We were only running into this for snap- with the current implementation (since we don't have list inconsistencies simulated yet index-N isn't an issue).
For the snap- I actually made it check for the case when whatever is written only differs in timstamps (on a master failover this can happen) and let it pass.
Shard level overwrites I made fail 100% of the time (see comment I added for that, it's not expected).
I think this is pretty neat now. This hasn't failed over 10k+ runs of SnapshotResiliencyTests and shows that we're handling master fail-overs reasonably safely on S3 (obviously not testing list inconsistencies yet ... but still :D).

...test/java/org/elasticsearch/snapshots/mockstore/MockEventuallyConsistentRepositoryTests.java

…ent-mock-repo

…ests

…ent-mock-repo

original-brownbear · 2019-07-16T14:06:21Z

@ywelsch thanks for taking a look. I fixed the dirty parts you pointed out and way enhanced the overwrite check + added test coverage for that now. Let me know what you think.

…ent-mock-repo

ywelsch

LGTM

original-brownbear · 2019-07-18T13:27:19Z

thanks @ywelsch !

…yTests (elastic#40893) * Add eventually consistent mock repository for reproducing and testing AWS S3 blob store behavior * Relates elastic#38941

…yTests (#40893) (#44570) * Add eventually consistent mock repository for reproducing and testing AWS S3 blob store behavior * Relates #38941

original-brownbear added 8 commits April 4, 2019 21:00

Add Eventually Consistent Mock Repository

6f24de1

* Reproduce S3 semantics exactly

bck

15fe43f

Merge remote-tracking branch 'elastic/master' into eventually-consist…

233ef74

…ent-mock-repo

better

c346c3d

repeat 5000

ab8da42

repeat 5000

8713682

Merge remote-tracking branch 'elastic/master' into eventually-consist…

6d7c993

…ent-mock-repo

bck

86e3d62

original-brownbear added >test Issues or PRs that are addressing/adding tests :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.0.0 v7.2.0 labels Apr 5, 2019

original-brownbear requested review from andrershov and ywelsch April 5, 2019 11:16

andrershov reviewed Apr 16, 2019

View reviewed changes

original-brownbear added 2 commits April 17, 2019 10:44

Merge remote-tracking branch 'elastic/master' into eventually-consist…

e7587cb

…ent-mock-repo

CR: Fix incorrect consistency and deleting of empty trees

e320ebc

original-brownbear requested a review from andrershov April 17, 2019 11:06

original-brownbear added 3 commits April 17, 2019 13:16

Merge remote-tracking branch 'elastic/master' into eventually-consist…

81a4ad6

…ent-mock-repo

CR: Fix incorrect consistency and deleting of empty trees

7b5c2e6

handle empty dirs like on S3/GCS

dfffe74

original-brownbear mentioned this pull request Apr 18, 2019

SnapshotResiliencyTests.testConcurrentSnapshotCreateAndDelete and testSnapshotPrimaryRelocations fail #41326

Closed

Merge remote-tracking branch 'elastic/master' into eventually-consist…

bc751fd

…ent-mock-repo

original-brownbear mentioned this pull request Apr 18, 2019

Fix SnapshotResiliencyTest Repo Consistency Check #41332

Merged

andrershov suggested changes Apr 19, 2019

View reviewed changes

original-brownbear added 2 commits April 19, 2019 12:27

Merge remote-tracking branch 'elastic/master' into eventually-consist…

ea12df2

…ent-mock-repo

CR: fix failing to use cached misses in read and exists calls

0f35d33

jpountz added v7.4.0 and removed v7.3.0 labels Jul 3, 2019

ywelsch reviewed Jul 12, 2019

View reviewed changes

original-brownbear added 4 commits July 15, 2019 10:40

Merge branch 'eventually-consistent-mock-repo' of github.com:original…

7d09033

…-brownbear/elasticsearch into eventually-consistent-mock-repo

Merge remote-tracking branch 'elastic/master' into eventually-consist…

c73f33b

…ent-mock-repo

Merge remote-tracking branch 'elastic/master' into eventually-consist…

3c94924

…ent-mock-repo

CR: Test, fix delete case, dry up ensure open

d1f1e79

original-brownbear requested a review from ywelsch July 16, 2019 06:07

ywelsch reviewed Jul 16, 2019

View reviewed changes

original-brownbear added 4 commits July 16, 2019 12:16

Merge remote-tracking branch 'elastic/master' into eventually-consist…

f00ba32

…ent-mock-repo

Merge remote-tracking branch 'elastic/master' into eventually-consist…

dc8bac3

…ent-mock-repo

CR: much stronger overwrite blob assertions, some cleanups, cleaner t…

58a9d42

…ests

Merge remote-tracking branch 'elastic/master' into eventually-consist…

7683dcc

…ent-mock-repo

original-brownbear requested a review from ywelsch July 16, 2019 14:06

original-brownbear added 2 commits July 17, 2019 09:23

Merge remote-tracking branch 'elastic/master' into eventually-consist…

07bc1cf

…ent-mock-repo

blobExists is gone :)

0afcaa3

ywelsch approved these changes Jul 18, 2019

View reviewed changes

original-brownbear merged commit 78e5cbb into elastic:master Jul 18, 2019

original-brownbear deleted the eventually-consistent-mock-repo branch July 18, 2019 13:28

original-brownbear added backport pending and removed backport pending labels Jul 18, 2019

original-brownbear mentioned this pull request Jul 18, 2019

Implement Eventually Consistent Mock Repository for SnapshotResiliencyTests (#40893) #44570

Merged

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Eventually Consistent Mock Repository for SnapshotResiliencyTests #40893

Implement Eventually Consistent Mock Repository for SnapshotResiliencyTests #40893

original-brownbear commented Apr 5, 2019

elasticmachine commented Apr 5, 2019

andrershov Apr 16, 2019

original-brownbear Apr 16, 2019

original-brownbear commented Apr 17, 2019

original-brownbear commented Apr 18, 2019

andrershov left a comment

ywelsch left a comment

ywelsch Jul 12, 2019

ywelsch Jul 12, 2019

ywelsch Jul 12, 2019

original-brownbear commented Jul 16, 2019

ywelsch left a comment

ywelsch Jul 16, 2019

ywelsch Jul 16, 2019

original-brownbear Jul 16, 2019

original-brownbear Jul 16, 2019

original-brownbear commented Jul 16, 2019

ywelsch left a comment

original-brownbear commented Jul 18, 2019

Implement Eventually Consistent Mock Repository for SnapshotResiliencyTests #40893

Implement Eventually Consistent Mock Repository for SnapshotResiliencyTests #40893

Conversation

original-brownbear commented Apr 5, 2019

elasticmachine commented Apr 5, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

original-brownbear commented Apr 17, 2019

original-brownbear commented Apr 18, 2019

andrershov left a comment

Choose a reason for hiding this comment

ywelsch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

original-brownbear commented Jul 16, 2019

ywelsch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

original-brownbear commented Jul 16, 2019

ywelsch left a comment

Choose a reason for hiding this comment

original-brownbear commented Jul 18, 2019