enable setting compression level

[PSeitz]

No description provided.

[fulmicoton]

why isn't that part of the compressor?

[PSeitz]

Compressor is currently constructed by deserializing from the compressor name. To pass more parameters we would need much more complicated deserialization.
And there are cases where the Compressor is only used for decompression.

[fulmicoton]

Why isn't compression part of the compressor?

If we have parameters that are compressor specific, it seems like an antipattern to have the union of these parameters in a list of options.

[codecov-commenter]

Codecov Report

Merging #1378 (834cd4d) into main (f0a2b1c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1378   +/-   ##
=======================================
  Coverage   94.22%   94.23%           
=======================================
  Files         235      235           
  Lines       42893    42922   +29     
=======================================
+ Hits        40418    40449   +31     
+ Misses       2475     2473    -2     

Impacted Files	Coverage Δ
src/core/index_meta.rs	93.02% <100.00%> (+0.03%)	⬆️
src/indexer/segment_serializer.rs	98.14% <100.00%> (+0.03%)	⬆️
src/indexer/segment_writer.rs	96.37% <100.00%> (+0.03%)	⬆️
src/store/compression_zstd_block.rs	90.47% <100.00%> (+1.00%)	⬆️
src/store/compressors.rs	87.09% <100.00%> (+2.19%)	⬆️
src/store/mod.rs	99.17% <100.00%> (ø)
src/store/writer.rs	99.00% <100.00%> (+0.09%)	⬆️
src/fastfield/reader.rs	89.91% <0.00%> (+0.84%)	⬆️
src/directory/watch_event_router.rs	96.63% <0.00%> (+0.84%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f0a2b1c...834cd4d. Read the comment docs.

[PSeitz]

Why isn't compression part of the compressor?

We have decompress only compressor

    /// Opens a store reader
    pub fn open(store_file: FileSlice) -> io::Result<StoreReader> {
        let (footer, data_and_offset) = DocStoreFooter::extract_footer(store_file)?;

        let (data_file, offset_index_file) = data_and_offset.split(footer.offset as usize);
        let index_data = offset_index_file.read_bytes()?;
        let space_usage = StoreSpaceUsage::new(data_file.len(), offset_index_file.len());
        let skip_index = SkipIndex::open(index_data);
        Ok(StoreReader {
            compressor: footer.compressor,
            data: data_file,
            cache: Arc::new(Mutex::new(LruCache::new(LRU_CACHE_CAPACITY))),
            cache_hits: Default::default(),
            cache_misses: Default::default(),
            skip_index: Arc::new(skip_index),
            space_usage,
        })
    }

If we have parameters that are compressor specific, it seems like an antipattern to have the union of these parameters in a list of options.

We could have zstd_compression_level or similar instead ofdocstore_compression_level, but I think we can use that parameter for multiple compressors. It's usually just a number.

[fulmicoton]

@PSeitz Can you investigate if a refactoring to make that nicer?

So the compressor enum would contain these params.
It then needs to be serialized/deserialized in the meta.json, as well as in the footer.

The footer part is a bit lame, because the level is not necessary, but that's fine.

[fulmicoton]

can it be negative?

[PSeitz]

Yes, for more speed