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

backport uuid identifier related changes to support uuid based sstable identifiers #333

Open
tchaikov opened this issue Jun 8, 2023 · 12 comments

Comments

@tchaikov
Copy link
Contributor

tchaikov commented Jun 8, 2023

uuid-based sstable identifier was introduced by Cassandra upstream in apache/cassandra@0040fea. but our fork was based 3.x. so to support this feature, we need to merge the upstream changes or rebase on top of it. or, as put by Avi in scylladb/scylladb#13932 (comment), quoted here as well

We should probably drop sstableloader. We might develop a standalone format converter from Cassandra formats to our format, based on the Cassandra codebase. So migration would use either the Spark migrator, or (Cassandra sstables -> Scylla sstables) -> load'n'stream. This gives us one less reason to keep having tools/java/.

Note that Cassandra is now at "oa" format: apache/cassandra@f16fb67.

without this change following test would be failing after enabling the uuid_sstable_identifier_enabled option introduced by scylladb/scylladb#13932

the tests above are only a subset. following tools should be updated

  • sstabledump
  • sstableloader
  • sstableverify
  • sstablemetadata
@tchaikov
Copy link
Contributor Author

tchaikov commented Jun 8, 2023

see also scylladb/scylladb#13932

@denesb
Copy link
Contributor

denesb commented Jun 14, 2023

I want to patch out sstabledump from all dtests. But I don't know when I will get around to do that, and certainly don't want to block the uuid migration on this.

@tchaikov
Copy link
Contributor Author

tchaikov commented Jun 14, 2023

@denesb hi Botond, thank you for the remarks! the recent changes of

can enable us to proceed without being blocked by this issue or your initiative to ditch sstabledump. and your change will allow revert some of these dtest changes.

@tchaikov tchaikov changed the title rebase against cassandra 4.x branch to support uuid based sstable identifiers backport uuid identifier related changes to support uuid based sstable identifiers Jun 14, 2023
@tchaikov
Copy link
Contributor Author

@denesb hi Botond, i am going to add a wrapper around scylla dump-data to replace sstabledump, what do you think? i think once we have scylladb/scylladb#14726 . we will be able to drop sstabledump.

@juliayakovlev
Copy link

sstablemetadata also fails on uuid-based sstable identifier

@tchaikov
Copy link
Contributor Author

sstablemetadata also fails on uuid-based sstable identifier

thank you. added to the list.

@tchaikov
Copy link
Contributor Author

an alternative of backport uuid identifier changes is to implement it right in scylla. see scylladb/scylladb#14856

fruch added a commit to fruch/scylla-cluster-tests that referenced this issue Aug 6, 2023
cause of new sstable uuid identifer feature the old `sstabledump`
can't currently work with those sstables

Ref: scylladb/scylla-tools-java#333
fruch added a commit to fruch/scylla-cluster-tests that referenced this issue Aug 6, 2023
cause of new sstable uuid identifer feature the old `sstabledump`
can't currently work with those sstables

Ref: scylladb/scylla-tools-java#333
fruch added a commit to scylladb/scylla-cluster-tests that referenced this issue Aug 7, 2023
cause of new sstable uuid identifer feature the old `sstabledump`
can't currently work with those sstables

Ref: scylladb/scylla-tools-java#333
fruch added a commit to scylladb/scylla-cluster-tests that referenced this issue Nov 14, 2023
cause of new sstable uuid identifer feature the old `sstabledump`
can't currently work with those sstables

Ref: scylladb/scylla-tools-java#333
(cherry picked from commit cd69a61)
@mykaul
Copy link
Contributor

mykaul commented Nov 23, 2023

@denesb hi Botond, i am going to add a wrapper around scylla dump-data to replace sstabledump, what do you think? i think once we have scylladb/scylladb#14726 . we will be able to drop sstabledump.

@tchaikov - is this still the plan?

@tchaikov
Copy link
Contributor Author

tchaikov commented Nov 23, 2023

@mykaul no, in an offline discussion with Botond, he warned me that the output format of scylla dump-data was different from that of sstabledump. so unless we translate the former into the latter in the wrapper i imagined, it's a no-go. so we are actively replacing sstabledump in our tests. and mark it deprecated, and then plan to remove it in favor of scylla sstable after a grace period.

@mykaul
Copy link
Contributor

mykaul commented Nov 23, 2023

@mykaul no, in an offline discussion with Botond, he warned me that the output format of scylla dump-data was different from that of sstabledump. so unless we translate the former into the latter in the wrapper i imagined, it's a no-go. so we are actively replacing sstabledump in our tests. and mark it deprecated, and then plan to remove it in favor of scylla sstable after a grace period.

I don't mind deprecating it, but it has docs implications. https://opensource.docs.scylladb.com/stable/operating-scylla/admin-tools/sstabledump.html for example.

@tchaikov
Copy link
Contributor Author

https://opensource.docs.scylladb.com/stable/operating-scylla/admin-tools/sstabledump.html

@mykaul hi Yaniv, please take a look at the master version https://opensource.docs.scylladb.com/master/operating-scylla/admin-tools/sstabledump.html . we are deprecating it.

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

No branches or pull requests

5 participants