-
Notifications
You must be signed in to change notification settings - Fork 699
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
SOLR-17537: Manage ZK Compression through Curator #2849
base: main
Are you sure you want to change the base?
Conversation
Could we call compress() universally and then our compressor chooses to only compress based on our criteria (state.json)? Just an idea off the top of my head; I don't know where compress() is called. |
are you suggesting that it just be a default? And so any data, on it's way in is compressed, and on it's way back out is uncompressed? Interesting idea! |
No, but it's confusing because I did say call
Thus no effective change from today. |
The idea is that with this we can eventually use the Curator recipes, which will require that our interactions with the CuratorFramework and SolrZkClient behave similarly. So yes, we could in the short term, just use |
f63d754
to
7450015
Compare
With the Curator Global Compression stuff merged, we should be good to go here. |
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.
This looks much nicer. I can't find it, but wasn't there one path that would need to check solr.xml
to decide if compression was happening or not? I guess that still happens in this code path, it's just that Curator manages things.
Yeah, I'm not changing how the configuration is done, just how it's handled in the backend. |
https://issues.apache.org/jira/browse/SOLR-17537
Unfortunately, Curator currently requires that users explicitly use (
.compress()
) in order to enable compression for a zNode. We need to add a feature in Curator that allows for a "CompressionFilter" or something like that.We will need to wait on a Curator release with that in it in order to merge this.