-
Notifications
You must be signed in to change notification settings - Fork 54
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
Disable xattrs by default #1133
Conversation
b8da58b
to
85f2e91
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM, thanks for carrying this forward. Let's add some more for the documentation of XAttrs in CLI and configuration.
c239579
to
f66d57b
Compare
69fcf67
to
1d2ca38
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, happy with how this turned out. Great work.
190561e
to
ca6bef6
Compare
ca6bef6
to
d49b88e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks.
Change --optimizations xattr to be default behavior, and add a new flag to disable this annotation when creating a SOCI index. This change also eliminated the need for the optimizations structure in the CLI. Signed-off-by: David Son <davbson@amazon.com>
d49b88e
to
56f57af
Compare
After some discussion, we decided to remove the TOML variable to ignore this at runtime. Instead, should users wish to force xattrs to be enabled, they must recreate a new SOCI index with the flag disabled. This is done for clarity on the developer side, and users are very unlikely to want to turn off this behavior anyway. |
Issue #, if available:
Followup to #1021
Description of changes:
#1021 introduced a check to not call
GetXAttrs()
and related calls, as well as building an index with these annotations. This behavior was opt-out by default. As it generally increases performance, this PR changes this to be default behavior.This also removes the need for the Optimizations structure in the CLI, thus it was removed.
Testing performed:
make test
. Additionally made some indexes and confirmed the flag created/didn't create the annotations based on expectation.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.