periodic compaction seconds not working as expected

[zaidoon1]

My understanding is that with periodic compaction seconds, we can schedule files to be compacted. And when compaction runs, tombstones are removed to keep db size small among other things.

My db size is small (around 5gb) so leaving all the defaults in place for rocksdb will mean I will accumulate a lot of tombstones so before trying to use periodic compactions seconds (because it wasn't exposed via c api so I couldn't use it), I used less optimal settings to force compaction:

opts.set_max_bytes_for_level_base(25 * 1024 * 1024);
opts.set_target_file_size_base(32 * 1024 * 1024);

which was fine but it's not ideal. After periodic compaction seconds was exposed via c api, I removed the two settings above and set periodic compaction seconds to run every 3 hours (i delete data on a cron every 4 hours).

As we can see, periodic compaction seconds doesn't seem to force compaction? or at least doesn't delete tombstones like I expected:

Here is the LOG file as well in case it helps:
LOG.txt

Any idea why periodic compaction seconds is not working? Or at least is not doing the same thing as setting the "bad" options

[cbi42]

periodic_compaction_seconds compacts file to the same level. If a file with tombstones is not in the bottommost level, it may not be helpful. You may want to set ttl too which compacts old files down.

You may want to consider using delete triggered compaction if the goal is to remove files with many tombstones:

rocksdb/include/rocksdb/utilities/table_properties_collectors.h

Line 18 in 7b24dec

class CompactOnDeletionCollectorFactory

i delete data on a cron every 4 hours

Another thing to consider to is do manual compaction after you issue deletions.

[zaidoon1]

oh so I did misunderstand what this feature does, what does compacts file to the same level achieve?

also I did read in the docs about setting ttl to compact old files down but I'm not sure what exact feature that is. Can you link me to the code for the compaction ttl? (specifically, I rely on the C api since i'm using rust rocksdb so hopefully that's exposed)

delete triggered compaction i

regarding this, I initially considered it but after asking around in the community, it seems this feature is best suited for when deletion is concentrated in a specific range where as my deletes are all over the place. Is this accurate?

Another thing to consider to is do manual compaction after you issue deletions.

This definitely sounds like a great idea and I'll likely go with this approach and maybe combine it with the ttl based compaction.

Also regarding https://github.com/facebook/rocksdb/wiki/Manual-Compaction#compactrange, do you recommend I change any of the defaults to achieve what I want of removing tombstones? I'm guessing I need to set CompactRangeOptions::bottommost_level_compaction to BottommostLevelCompaction::KforceOptimized since I don't have a custom compaction filter setup and I want to compact the bottommost level?

[cbi42]

what does compacts file to the same level achieve

It lets files go through compaction filter periodically. The option has a different meaning for universal compaction.

Can you link me to the code for the compaction ttl?

I don't see it in C API. Feel free to submit a PR to add it.

delete triggered compaction

You can specify a file to be compacted if it has more than a D tombstones in a window of X entries, or that the ratio of tombstones exceed some value.

I'm guessing I need to set CompactRangeOptions::bottommost_level_compaction to BottommostLevelCompaction::KforceOptimized

Yeah, setting it to kForceOptimized should be enough.

[ajkr]

I wonder if we could get away with the following behavior change: in leveled compaction, also trigger compaction to the next level for upper-level files whose data age exceeds periodic_compaction_seconds. (Is periodic_compaction_seconds < ttl in leveled compaction a common/useful setting today?)

[git-hulk]

I wonder if we could get away with the following behavior change: in leveled compaction, also trigger compaction to the next level for upper-level files whose data age exceeds periodic_compaction_seconds.

As a user, I believe it would be better if we could trigger compaction to the next level. I can submit a PR to resolve this if it sounds good to maintainers.

[zaidoon1]

I also agree that it would be great to trigger compaction to the next level.

[zaidoon1]

I've created a pr to expose options ttl via c api #12170

[git-hulk]

Is periodic_compaction_seconds < ttl in leveled compaction a common/useful setting today?

I see, seems periodic_compaction_seconds will perform the same way with TTL in level compaction if allowed to trigger next-level compaction.

[zaidoon1]

right, based on my understanding, I completely misunderstood how periodic_compaction_seconds works, what I was looking for is the TTL option

[git-hulk]

I checked the implementation, and the TTL option only triggers the next-level compaction without caring about the input file level. So I also think it's good to switch to the TTL option since we're using the level compaction.

[cbi42]

Is periodic_compaction_seconds < ttl in leveled compaction a common/useful setting today?

Probably not, this can also prevent ttl compaction from happening.

trigger compaction to the next level for upper-level files whose data age exceeds periodic_compaction_seconds

I'm fine with this change. It should not affect users who configure ttl <= periodic_compaction_seconds. The change may not make sense when level_compaction_dynamic_level_bytes=false, but neither does ttl.

[ajkr]

As a user, I believe it would be better if we could trigger compaction to the next level. I can submit a PR to resolve this if it sounds good to maintainers.

It sounds good to me

[git-hulk]

I'm fine with this change. It should not affect users who configure ttl <= periodic_compaction_seconds. The change may not make sense when level_compaction_dynamic_level_bytes=false, but neither does ttl.

Sure, thank you!

[kishorenc]

I just stumbled on this exact same gotcha. Perhaps the documentation / comments around periodic_compaction_seconds could have been clearer in this regard. Like others on this thread, I fully expected compaction to trigger to the next level and that would be a welcome change.

[cbi42]

periodic_compaction_seconds was working as expected and improvement is made in #12175.