Optimize segmentwise recompression #7482
Conversation
|
Do we have benchmarks to confirm that this is faster? |
antekresic
force-pushed
the
antekresic/optimize_recompression
branch
4 times, most recently
from
a1b8376 to
ae32773
Compare
antekresic
added
compression
enhancement
| * 4: frozen | ||
| * 8: compressed_partial | ||
| */ | ||
| if (!ts_chunk_is_compressed(uncompressed_chunk) && ts_chunk_is_partial(uncompressed_chunk)) |
There was a problem hiding this comment.
This condition reads a little weird, how can chunk be uncompressed and partially compressed at the same time? Might be not a topic for this PR, but we might have to rename ts_chunk_is_compressed to something like ts_chunk_is_fully_compressed.
There was a problem hiding this comment.
Yeah, that check seems to check for two bitfields which should always accompany each other (compressed and partially compressed). Having partially compressed without the compressed bit should be an internal error.
Maybe Ensure is more suitable for this check?
There was a problem hiding this comment.
I'll do this in a follow-up PR since its the existing state which I've just moved to a separate file.
| * 8: compressed_partial | ||
| */ | ||
| if (!ts_chunk_is_compressed(uncompressed_chunk) && ts_chunk_is_partial(uncompressed_chunk)) | ||
| elog(ERROR, |
There was a problem hiding this comment.
Looks like this still can be triggered by a user calling it for an unsuitable chunk? Needs better errcode then.
There was a problem hiding this comment.
No, this shouldn't not happen in regular execution. That's why I think Ensure would be better suited here.
antekresic
force-pushed
the
antekresic/optimize_recompression
branch
2 times, most recently
from
a560496 to
c0c2c14
Compare
| tsl_recompress_chunk_segmentwise(PG_FUNCTION_ARGS) | ||
| { | ||
| Oid uncompressed_chunk_id = PG_ARGISNULL(0) ? InvalidOid : PG_GETARG_OID(0); | ||
| bool if_not_compressed = PG_ARGISNULL(1) ? true : PG_GETARG_BOOL(1); |
There was a problem hiding this comment.
The default is true here while in the function comment it is stated as false
There was a problem hiding this comment.
Will address this in a follow-up PR since its current state and has very little to do with the change here.
antekresic
force-pushed
the
antekresic/optimize_recompression
branch
from
c0c2c14 to
4b4d270
Compare
With the recent change to compression schema, we can optimize segmentwise recompression and target only batches which are affected to be recompressed or compress tuples which don't belong to any batch without any decompression. This means narrowing down the amount of that we need to decompress and recompress, ultimately speeding up the operation and generating less bloat in the compressed chunk.
antekresic
force-pushed
the
antekresic/optimize_recompression
branch
from
4b4d270 to
14f528c
Compare
With the recent change to compression schema, we can optimize segmentwise recompression and target only batches which are affected to be recompressed or compress tuples which don't belong to any batch without any decompression. This means narrowing down the amount of that we need to decompress and recompress, ultimately speeding up the operation and generating less bloat in the compressed chunk.
Benchmark with improvements (2-3x in certain cases):
https://grafana.ops.savannah-dev.timescale.com/d/fasYic_4z/compare-akuzm?orgId=1&var-branch=All&var-run1=3959&var-run2=3960&var-threshold=0&var-use_historical_thresholds=true&var-threshold_expression=2.5%20%2A%20percentile_cont%280.90%29&var-exact_suite_version=false