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.

Sign up for GitHub

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

Jump to bottom

scylla-sstable ignores scylla.yaml unless it is explicitely provided #16132

Closed
denesb opened this issue Nov 22, 2023 · 3 comments
Closed

scylla-sstable ignores scylla.yaml unless it is explicitely provided #16132

denesb opened this issue Nov 22, 2023 · 3 comments
Assignees
@denesb
Labels
area/scylla tools
Milestone
6.0

Comments

@denesb
Copy link
Contributor

denesb commented Nov 22, 2023

scylla-sstable tries multiple things to obtain the schema of the sstable(s) it is provided. The order of things it tries is as follows:

  • read schema from ./schema.cql
  • read schema from system table path, derived from the path of the sstable
  • read scylla.yaml from the default location and obtain ScyllaDB's data path from it
  • read schema from default data path

scylla-sstable will stop immediately if one of these is successful. This is fine for the most path, but it provides a somewhat clunky UX on enterprise, when EAR is used. I won't go into much detail, given that this is an enterprise feature, the point is that for encrypted sstables to be read, scylla.yaml has to be processed. To make the tool work OOTB with EAR, it should always attempt to read scylla.yaml. To minimize the difference between the enterprise and open-source code-base, this change should be made in open-source too. It is also a forward-looking change, in case OSS will ever require the configuration read, to correctly process sstables.
@denesb denesb added this to the 6.0 milestone Nov 22, 2023
@denesb denesb self-assigned this Nov 22, 2023
@denesb denesb mentioned this issue Nov 22, 2023
Merged
10 tasks
denesb added a commit to denesb/scylla that referenced this issue Nov 24, 2023
@denesb
tools/scylla-sstable: always read scylla.yaml
0d232d7 
Fixes: scylladb#16132
denesb added a commit to denesb/scylla that referenced this issue Nov 24, 2023
@denesb
tools/scylla-sstable: always read scylla.yaml
226710c 
Currently, scylla.yaml is read conditionally, if either the user
provided `--scylla-yaml-file` command line parameter, or if deducing the
data dir location from the sstable path failed.
We want the scylla.yaml file to be always read, so that when working
with encrypted file (enterprise), scylla-sstable can pick up the
configuration for the encryption.
This patch makes scylla-sstable always attempt to read the scylla-yaml
file, whether the user provided a location for it or not. When not, the
default location is used (also considering the `SCYLLA_CONF` and
`SCYLLA_HOME` environment variables.
Failing to find the scylla.yaml file is not considered an error. The
rational is that the user will discover this if they attempt to do an
operation that requires this anyway.
There is a debug-level log about whether it was successfully read or
not.

Fixes: scylladb#16132
@denesb denesb mentioned this issue Nov 24, 2023
Closed
fruch added a commit to fruch/scylla-cluster-tests that referenced this issue Dec 20, 2023
@fruch
fix(upgrade_test): scylla sstable dump verify with scylla.yaml
439fb28 
since there a fix not yet backported about the order that `scylla
sstable dumps` is looking for the schema, we need to pass it
the scylla.yaml for it to be able to read EaR sstables

Ref: scylladb/scylla-enterprise#3702
Ref: scylladb/scylladb#16132
@fruch fruch mentioned this issue Dec 20, 2023
Merged
8 tasks
fruch added a commit to fruch/scylla-cluster-tests that referenced this issue Dec 21, 2023
@fruch
fix(upgrade_test): scylla sstable dump verify with scylla.yaml
cee268f 
since there a fix not yet backported about the order that `scylla
sstable dumps` is looking for the schema, we need to pass it
the scylla.yaml for it to be able to read EaR sstables

Ref: scylladb/scylla-enterprise#3702
Ref: scylladb/scylladb#16132
fruch added a commit to scylladb/scylla-cluster-tests that referenced this issue Dec 24, 2023
@fruch
fix(upgrade_test): scylla sstable dump verify with scylla.yaml
5aad817 
since there a fix not yet backported about the order that `scylla
sstable dumps` is looking for the schema, we need to pass it
the scylla.yaml for it to be able to read EaR sstables

Ref: scylladb/scylla-enterprise#3702
Ref: scylladb/scylladb#16132
fruch added a commit to scylladb/scylla-cluster-tests that referenced this issue Dec 24, 2023
@fruch
fix(upgrade_test): scylla sstable dump verify with scylla.yaml
07187dd 
since there a fix not yet backported about the order that `scylla
sstable dumps` is looking for the schema, we need to pass it
the scylla.yaml for it to be able to read EaR sstables

Ref: scylladb/scylla-enterprise#3702
Ref: scylladb/scylladb#16132
(cherry picked from commit 5aad817)
@mykaul mykaul added the backport/5.4 Issues that should be backported to 5.4 branch once they'll be fixed label Jan 4, 2024
@denesb
Copy link
Contributor Author

denesb commented Jan 4, 2024

There is no need to backport this to any OSS release, only enterprise is affected by this patch.

@denesb denesb removed Backport candidate backport/5.4 Issues that should be backported to 5.4 branch once they'll be fixed labels Jan 4, 2024
@rayakurl
Copy link
Contributor

@fruch does it mean we can use scylla sstable instead of sstable dump in boyk tests?

@fruch
Copy link
Contributor

fruch commented Jan 14, 2024

@fruch does it mean we can use scylla sstable instead of sstable dump in boyk tests?

this is just one of a list of issues that are tracked in:
https://github.com/scylladb/scylla-enterprise/issues/3716

dgarcia360 pushed a commit to dgarcia360/scylla that referenced this issue Apr 30, 2024
@denesb @dgarcia360
tools/scylla-sstable: always read scylla.yaml
72584a9 
Currently, scylla.yaml is read conditionally, if either the user
provided `--scylla-yaml-file` command line parameter, or if deducing the
data dir location from the sstable path failed.
We want the scylla.yaml file to be always read, so that when working
with encrypted file (enterprise), scylla-sstable can pick up the
configuration for the encryption.
This patch makes scylla-sstable always attempt to read the scylla-yaml
file, whether the user provided a location for it or not. When not, the
default location is used (also considering the `SCYLLA_CONF` and
`SCYLLA_HOME` environment variables.
Failing to find the scylla.yaml file is not considered an error. The
rational is that the user will discover this if they attempt to do an
operation that requires this anyway.
There is a debug-level log about whether it was successfully read or
not.

Fixes: scylladb#16132

Closes scylladb#16174
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@denesb denesb

Labels
area/scylla tools
Projects
None yet
Milestone
6.0
Development

Successfully merging a pull request may close this issue.

tools/scylla-sstable: always read scylla.yaml denesb/scylla
5 participants
@fruch @denesb @mykaul @scylladb-promoter @rayakurl