Support weaker consistency model for S3 MPUs #138663
Conversation
Adjusts the implementation of linearizable registers in S3 repositories to allow for the weaker multipart upload API semantics observed in practice. Also adjusts the S3 test fixture to (optionally) simulate the weaker semantics, and extends the repository analysis REST tests to cover both cases.
DaveCTurner
added
>bug
:Distributed Coordination/Snapshot/Restore
elasticsearchmachine
added
the
Team:Distributed Coordination
|
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
|
Hi @DaveCTurner, I've created a changelog YAML for you. |
🔍 Preview links for changed docs |
ℹ️ Important: Docs version tagging👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version. We use applies_to tags to mark version-specific features and changes. Expand for a quick overviewWhen to use applies_to tags:✅ At the page level to indicate which products/deployments the content applies to (mandatory) What NOT to do:❌ Don't remove or replace information that applies to an older version 🤔 Need help?
|
Reflects the changes introduced in elastic/elasticsearch#138663.
joshua-adams-1
left a comment
joshua-adams-1
left a comment
There was a problem hiding this comment.
Looks good to me overall, just had a few small comments
| package fixture.s3; | ||
|
|
||
| /** | ||
| * AWS S3 has weaker consistency for its multipart upload APIs than initially claimed (see support cases 10837136441 and 176070774900712) |
There was a problem hiding this comment.
Side note, are these weaker consistencies to be fixed, or "features" now?
| * The alternative model verified by these tests: the multipart upload APIs are strongly consistent, but the {@code If-Match} and | ||
| * {@code If-None-Match} headers are ignored and all writes are unconditional. | ||
| */ | ||
| STRONG_MPUS(false, true); |
There was a problem hiding this comment.
Is it possible for a model to have both?
| } | ||
|
|
||
| protected S3ConsistencyModel consistencyModel() { | ||
| return S3ConsistencyModel.AWS_DEFAULT; |
There was a problem hiding this comment.
This value is set in both the S3HttpHandler and the S3HttpFixture. Is there a way to share it between classes?
| } | ||
| } | ||
|
|
||
| public static void runInParallel(Runnable... tasks) { |
There was a problem hiding this comment.
[Nit] Missing Javadoc
| } | ||
|
|
||
| protected S3ConsistencyModel consistencyModel() { | ||
| return S3ConsistencyModel.AWS_DEFAULT; |
There was a problem hiding this comment.
Is there a benefit in randomising this between AWS_DEFAULT and STRONG_MPUS? (Not here, because we'd want multiple consistencyModel calls to be consistent, but as a class variable?). Would this remove the need to have the S3RepositoryAnalysisStrongMpusRestIT?
3 participants
Adjusts the implementation of linearizable registers in S3 repositories
to allow for the weaker multipart upload API semantics observed in
practice.
Also adjusts the S3 test fixture to (optionally) simulate the weaker
semantics, and extends the repository analysis REST tests to cover both
cases.