compact/planner: fix issue 6775

[GiedriusS]

It doesn't make sense to vertically compact downsampled blocks so mark them with the no compact marker if downsampled blocks were detected in the plan. Seems like the Planner is the best place for this logic - I just repeated the previous pattern with the large index file filter.

Closes #6775 .

[yeya24]

Instead of creating a new planner and excluding downsampled blocks from planning time, why we cannot filter out downsampled blocks from metasByMinTime when we pass blocks to the block?

I understand that compaction should only happen for raw blocks. Is there a usecase to compact downsampled blocks or compact downsampled blocks with raw blocks? If we do this, downsampled blocks will turn into raw blocks again, and we need to downsampled it again? I feel this is not ideal.

[GiedriusS]

Instead of creating a new planner and excluding downsampled blocks from planning time, why we cannot filter out downsampled blocks from metasByMinTime when we pass blocks to the block?

I understand that compaction should only happen for raw blocks. Is there a usecase to compact downsampled blocks or compact downsampled blocks with raw blocks? If we do this, downsampled blocks will turn into raw blocks again, and we need to downsampled it again? I feel this is not ideal.

I am inclined to agree but I am afraid that there will be some weird use-case out there that depends on compacting downsampled blocks. Maybe we could go with the current change as-is for the next release and then in $nextrelease+1 I will do as you've suggested?

[GiedriusS]

I deployed this in prod and it works. 😄

[nicolastakashi]

I also deployed this in prod and it's working like a charm 🥳

[yeya24]

I am ok to have the no compact marker first.

[yeya24]

I am inclined to agree but I am afraid that there will be some weird use-case out there that depends on compacting downsampled blocks

Let me think about this more. Downsampled blocks can be compacted with downsampled blocks with the same resolution only since they are in the same compaction group. It sounds possible to happen in the current planner creating a plan compacting downsampled blocks together. But I can only see blocks getting compacted into a raw block and the raw block getting downsampled again.

When two downsampled blocks overlap with each other, then it is probably valid to do a vertical compaction and merge them into 1 block. Is it possible to create a downsampled block directly rather than compacting a raw block first then get it downsampled again?

[GiedriusS]

I am inclined to agree but I am afraid that there will be some weird use-case out there that depends on compacting downsampled blocks

Let me think about this more. Downsampled blocks can be compacted with downsampled blocks with the same resolution only since they are in the same compaction group. It sounds possible to happen in the current planner creating a plan compacting downsampled blocks together. But I can only see blocks getting compacted into a raw block and the raw block getting downsampled again.

When two downsampled blocks overlap with each other, then it is probably valid to do a vertical compaction and merge them into 1 block. Is it possible to create a downsampled block directly rather than compacting a raw block first then get it downsampled again?

Unless two downsampled and vertically overlapping blocks are exactly the same, it is impossible to do vertical compaction on them. For example, it would be impossible to calculate AggrCount from two non-identical values.

I agree with you and I'm not 100% certain how this could happen but Filip had an idea that it is maybe related to the "index size too big" filter which could cause blocks to have "gaps" in them due to some blocks being filtered from compaction. Then, the downsampling part probably picks up those blocks that had been left out from compaction. And we end up with this situation.

[yeya24]

Then, the downsampling part probably picks up those blocks that had been left out from compaction. And we end up with this situation.

We can merge this pr first and think about not compacting downsampled blocks at all in next release.

[yeya24]

@GiedriusS Let's add changelog?

[GiedriusS]

@yeya24 added

[lu-xiansheng]

Can we filter out the blocks that are already fully compressed and then downsample them. This way downsampled blocks will not be compressed.There shouldn't be any other problems with this, right?