FrankChen021 commented on PR #20382: URL: https://github.com/apache/druid/pull/20382#issuecomment-5770315194
> > In this PR(and other object storage like deep storage), the new property druid.storage.zip is used to determine whether segment files are compressed as index.zip or not. However, in #18982 , we introduced a new property druid.storage.compressionFormat for HDFS deep storage to indicate which compression format is used for compression(zip and lz4), that PR didn't bring the change to object storage like S3, Azure ang google cloud(because we don't use object storage), but architecturally, that property should apply to all deep storages. So I think we should use the druid.storage.compressionFormat( with a new NONE enum supported ) to replace the druid.storage.zip. > > My stance is that it would make sense to add `druid.storage.compressionFormat` to `DeepStorageSegmentConfig`, but I didn't personally have much interest in adding support for it to the other deep storage implementations because I am primarily interested in storing V10 segment format files in deep storage uncompressed so that `SegmentRangeReader` can do partial reads to support partial loads on historicals. V10 format stores the segment as a single file (metadata header + a bunch of concatenated internal files so kind similar in spirit to zip 0) so it shouldn't change the overall number of files in deep storage (at least without any extensions to add/attach 'external' files). > > In terms of compression/encoding, going forward I am much more interested in ways to shrink the internal files stored inside the v10 segment file so that we can preserve the ability to do partial reads, so I'll be spending any time/energy I have to pursue that instead of expanding the types of generic compression we can apply to the whole container > I agree with you that as you're putting effort on the new segment file format, the new compression does not make sense, we also don't have any plan/interest to push these forward. To make current configuration design consistency across all deep storages, for this part, my position is that we can do a small clean up to deprecate the `druid.storage.zip` and use `druid.storage.compressionFormat` instead to eliminate ambigulty. If you don't have time, I can do this after this is merged. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
