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

docs: add documentation for scylla_identifier #21221

Closed
wants to merge 1 commit into from

Conversation

bhalevy
Copy link
Member

@bhalevy bhalevy commented Oct 22, 2024

Commit 3a12ad9
added an sstable_identifier uuid to the SSTable
scylla_metadata component, however it was
under-documented and this patch adds the missing
documentation for the sstable component format,
and to the scylla sstable tool documentation.

  • sstable_identifier is confined to master so no backport is needed

@bhalevy bhalevy added documentation Requires documentation backport/none Backport is not required labels Oct 22, 2024
@scylladb-promoter
Copy link
Contributor

Docs Preview 📖

Docs Preview for this pull request is available here

Changed Files:

Note: This preview will be available for 30 days and will be automatically deleted after that period. You can manually trigger a new build by committing changes.

change if the sstable is migrated to a different shard or node, the sstable
identifier is stable and copied with the rest of the scylla metadata.

The :doc:`scylla sstable dump-scylla-metadata </operating-scylla/admin-tools/scylla-sstable/#dump-scylla-metadata>` tool
Copy link
Collaborator

Choose a reason for hiding this comment

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

The docs directive will not work in an .md file. In the internal documents within the dev folder, it's better to use the markdown syntax to add links to the file on GitHub rather than the published docs. That would be:

[scylla sstable dump-scylla-metadata](https://github.com/scylladb/scylladb/blob/master/docs/operating-scylla/admin-tools/scylla-sstable.rst#dump-scylla-metadata)

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed in next version (a22bc02)

Commit 3a12ad9
added an sstable_identifier uuid to the SSTable
scylla_metadata component, however it was
under-documented and this patch adds the missing
documentation for the sstable component format,
and to the scylla sstable tool documentation.

Signed-off-by: Benny Halevy <[email protected]>
@bhalevy
Copy link
Member Author

bhalevy commented Oct 23, 2024

In a22bc02: use markdown syntax in .md doc (#21221 (comment))

@bhalevy bhalevy added the status/merge candidate Item needs maintainer attention label Oct 23, 2024
@bhalevy
Copy link
Member Author

bhalevy commented Oct 23, 2024

@scylladb/scylla-maint please consider merging

@tchaikov
Copy link
Contributor

@scylladb/scylla-maint hello maintainers, could you help queue this change?

@@ -500,6 +500,7 @@ The content is dumped in JSON, using the following schema:
"scylla_build_id": String
"scylla_version": String
"ext_timestamp_stats": {"$key": int64, ...}
"sstable_identifier": String, // UUID
Copy link
Member

Choose a reason for hiding this comment

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

Why is it a string and not a uuid?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is stored as a uuid.
I just followed run_identifier's example in the documentation.

Copy link
Member

Choose a reason for hiding this comment

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

Then it's misleading. String -> formatted UUID.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is documenting a JSON schema and JSON doesn't have a UUID type, so it has to be converted to string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/none Backport is not required documentation Requires documentation promoted-to-master status/merge candidate Item needs maintainer attention
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants