sarama icon indicating copy to clipboard operation
sarama copied to clipboard

Sarama w/ zstd trained dictionaries

Open seizethedave opened this issue 3 years ago • 6 comments

We are researching the use of zstd training dictionaries in our Kafka stack, and judging by the code, if Sarama decompresses a zstd payload with a specified dictionaryID, it will just fail, because Sarama statically calls zstd.NewReader() without the options where one can specify dictionaries. Am I reading this right, or is there an opportunity to inject the dictionaries somewhere?

Thanks!

seizethedave avatar May 20 '21 17:05 seizethedave

@seizethedave thanks for getting in touch. That sounds like interesting research and I'd be keen to learn more about your approach and subsequent results if you might be able to write it up in a short blog article somewhere?

Sadly you are correctly and currently Sarama doesn't expose any of the configuration options around zstd. I partially raised this under https://github.com/Shopify/sarama/issues/1937 as something we need to look at addressing, so I would certainly include WithDecoderDicts in the set of options we should ensure we are exposing to users.

I'm not sure what sort of timescale you're wanting to get this done within, but a PR would be welcome if you want to look at drafting something up for us to review and help you refine? Otherwise we can see what we can accomodate over the coming weeks.

dnwe avatar May 20 '21 22:05 dnwe

Thanks @dnwe. We are still vetting the use of dictionaries, so not super urgent. It seems to me that an extensible encoder/decoder plug-in system might be the best for all parties, as those using dictionaries will all have specific constraints to work under. (e.g., we will probably want to load new dicts at runtime.) That would also open the gate to people dropping in other compressor libs etc.

we’d love to contribute, will check in with you a bit later when we’re further along.

seizethedave avatar May 21 '21 17:05 seizethedave

Thank you for taking the time to raise this issue. However, it has not had any activity on it in the past 90 days and will be closed in 30 days if no updates occur. Please check if the main branch has already resolved the issue since it was raised. If you believe the issue is still valid and you would like input from the maintainers then please comment to ask for it to be reviewed.

github-actions[bot] avatar Aug 29 '23 18:08 github-actions[bot]

@seizethedave how are you getting on?

dnwe avatar Aug 29 '23 19:08 dnwe

@dnwe - Still seems valuable, but I left the org where I wanted to do that thing. Don't think they needed to do that, either.

seizethedave avatar Nov 11 '23 23:11 seizethedave

Thank you for taking the time to raise this issue. However, it has not had any activity on it in the past 90 days and will be closed in 30 days if no updates occur. Please check if the main branch has already resolved the issue since it was raised. If you believe the issue is still valid and you would like input from the maintainers then please comment to ask for it to be reviewed.

github-actions[bot] avatar Feb 10 '24 00:02 github-actions[bot]