Skip to content

Add checksum to S3 Anchor and change clone template as file-constant#22819

Open
JasMehta08 wants to merge 2 commits into
root-project:masterfrom
JasMehta08:s3-anchor-checksum-clone-template
Open

Add checksum to S3 Anchor and change clone template as file-constant#22819
JasMehta08 wants to merge 2 commits into
root-project:masterfrom
JasMehta08:s3-anchor-checksum-clone-template

Conversation

@JasMehta08

@JasMehta08 JasMehta08 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

This Pull request:

(Is a part of the GSoC 2026 project S3 Backend for RNTuple.)

Adds checksum and a configurable clone path to the S3 Anchor.

Changes or fixes:

  • Add an XXH3 64-bit checksum to the S3 anchor, computed over a canonical compact JSON representation of all data fields. Verified on read.
  • Replace the hardcoded clone path in CloneAsHidden with a file-local constant (kDefaultCloneTemplate).

Checklist:

  • tested changes locally

Comment thread tree/ntuple/inc/ROOT/RPageStorageS3.hxx Outdated
Comment thread tree/ntuple/inc/ROOT/RPageStorageS3.hxx Outdated
@JasMehta08
JasMehta08 force-pushed the s3-anchor-checksum-clone-template branch from 1373142 to 5f539d7 Compare July 15, 2026 12:56
Comment thread tree/ntuple/inc/ROOT/RPageStorageS3.hxx Outdated
Comment thread tree/ntuple/test/ntuple_storage_s3.cxx Outdated
Comment thread tree/ntuple/test/ntuple_storage_s3.cxx Outdated
@JasMehta08
JasMehta08 requested a review from silverweed July 15, 2026 13:33
@JasMehta08
JasMehta08 force-pushed the s3-anchor-checksum-clone-template branch from 5f539d7 to bfe8630 Compare July 18, 2026 05:46
@JasMehta08 JasMehta08 changed the title Add checksum and clone template to S3 Anchor Add checksum to S3 Anchor and Clone template as file-constant Jul 18, 2026
@JasMehta08 JasMehta08 changed the title Add checksum to S3 Anchor and Clone template as file-constant Add checksum to S3 Anchor and change clone template as file-constant Jul 18, 2026
@JasMehta08
JasMehta08 force-pushed the s3-anchor-checksum-clone-template branch from bfe8630 to 5945e72 Compare July 18, 2026 06:14
@JasMehta08

Copy link
Copy Markdown
Contributor Author

@silverweed and @jblomer

I have removed the clone template from the anchor, and made it a file-constant, please have a new review of the commits

Comment thread tree/ntuple/src/RPageStorageS3.cxx Outdated
/// the stored JSON uses pretty-printing for readability.
std::string ROOT::Experimental::Internal::RNTupleAnchorS3::ToJSON() const
{
auto canonical = CanonicalPayload(*this);

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.

Aren't we duplicating quite a bunch of work here? I get we need to serialize the anchor to compute the checksum and then add it to the JSON representation, but can't we add the checksum via string manipulation? (replacing canonical's last } with ,"checksum":...).

@JasMehta08 JasMehta08 Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, we can remove the use of CanonicalPayload from ToJSoN(), if we can just use the jsonAnchor to create the checksum and add the field at the end. I will update it to do so.

@github-actions

Copy link
Copy Markdown

Test Results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit 5945e72.

@JasMehta08
JasMehta08 force-pushed the s3-anchor-checksum-clone-template branch 2 times, most recently from 1d7e1db to 5b189a8 Compare July 20, 2026 07:22
@JasMehta08
JasMehta08 force-pushed the s3-anchor-checksum-clone-template branch from 5b189a8 to 06b9980 Compare July 20, 2026 08:47
@JasMehta08

JasMehta08 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

@silverweed ,

I have updated the implementation, and removed CanonicalPayload() from both ToJSoN() as well as CreateFromJSoN(). I removed the duplicate work that was being done by it. ToJSoN() using the anchor itself to create and checksum, and attaches it at the end. In CreateFromJSoN() it strips the checksum of the anchor, then using the remaining anchor to create a new hash and check against the checksum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants