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

Issue 6699: LTS - Prevent relocation of huge data with truncate when cluster is configured with very large max chunk size. #6700

Merged

Conversation

sachin-j-joshi
Copy link
Contributor

@sachin-j-joshi sachin-j-joshi commented Apr 15, 2022

Signed-off-by: Sachin Joshi sachin.joshi@emc.com

Change log description
Issue 6699: LTS - Prevent relocation of huge data with truncate when cluster is configured with very large max chunk size.

Purpose of the change
Fixes #6699

What the code does
Adds config value for maximum size of chunk after which it is not eligible for relocation.

How to verify it
All tests should pass

…cluster is configured with very large max chunk size.

Signed-off-by: Sachin Joshi <sachin.joshi@emc.com>
}, chunkedSegmentStorage.getExecutor());
}
return CompletableFuture.completedFuture(null);
}

private CompletableFuture<Void> copyBytes(ChunkHandle writeHandle, ChunkHandle readHandle, long startOffset, long length) {
Preconditions.checkArgument(length <= chunkedSegmentStorage.getConfig().getMaxSizeForTruncateRelocationInbytes(),
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if this precondition check fails? Isn't this going to relocate the chunk? what are the consequences of that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Precondition checks as name suggests check the invariant and prevent bad execution and unwanted side effects in case input is not valid. Therefore this will throw an exception if length is not less than max allowed.

Code elsewhere (below on line 229) should mean we should never in reality invoke copyBytes with invalid arguments. But assuming that through some bizarre overflow magic we somehow ended up here with bad parameter, then this precondition will fail and prevent further damage.

SLTS in general uses "fail fast" defensive programming approach everywhere and therefore immediately stops processing to prevent further damage.

@codecov
Copy link

codecov bot commented Apr 18, 2022

Codecov Report

Merging #6700 (18192d2) into master (213576b) will decrease coverage by 0.00%.
The diff coverage is 90.00%.

@@             Coverage Diff              @@
##             master    #6700      +/-   ##
============================================
- Coverage     86.30%   86.30%   -0.01%     
- Complexity    15738    15743       +5     
============================================
  Files          1022     1022              
  Lines         58852    58862      +10     
  Branches       5960     5961       +1     
============================================
+ Hits          50791    50798       +7     
+ Misses         4952     4950       -2     
- Partials       3109     3114       +5     
Impacted Files Coverage Δ
...entstore/storage/chunklayer/TruncateOperation.java 94.71% <85.71%> (-0.32%) ⬇️
...torage/chunklayer/ChunkedSegmentStorageConfig.java 100.00% <100.00%> (ø)
...ega/controller/store/task/ZKTaskMetadataStore.java 82.60% <0.00%> (-2.90%) ⬇️
...egmentstore/server/containers/MetadataCleaner.java 87.32% <0.00%> (-2.82%) ⬇️
...o/pravega/client/stream/impl/ReaderGroupState.java 92.04% <0.00%> (-2.17%) ⬇️
...ga/client/connection/impl/TcpClientConnection.java 89.72% <0.00%> (-1.37%) ⬇️
...o/pravega/common/cluster/zkImpl/ClusterZKImpl.java 70.45% <0.00%> (-1.14%) ⬇️
...tore/server/containers/StreamSegmentContainer.java 88.96% <0.00%> (-0.65%) ⬇️
.../segmentstore/server/containers/MetadataStore.java 87.76% <0.00%> (-0.43%) ⬇️
...ga/controller/task/Stream/StreamMetadataTasks.java 85.89% <0.00%> (-0.33%) ⬇️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 213576b...18192d2. Read the comment docs.

@RaulGracia RaulGracia merged commit 22e610a into pravega:master Apr 18, 2022
sachin-j-joshi added a commit to sachin-j-joshi/pravega that referenced this pull request Apr 18, 2022
…cluster is configured with very large max chunk size. (pravega#6700)

Prevent relocation of huge data with truncate when cluster is configured with very large max chunk size.

Signed-off-by: Sachin Joshi <sachin.joshi@emc.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LTS - Prevent relocation of huge data with truncate when cluster is configured with very large chunk size
3 participants