Enable rocksdb read option avoid_unnecessary_blocking_io to avoid unexpected long latency

[wanghenshui]

see https://github.com/facebook/rocksdb/wiki/IO#avoid-blocking-io

[git-hulk]

LGTM, thank you!

[mapleFU]

• https://github.com/facebook/rocksdb/blob/324453e5790f52557d02abf64a2dcadbaa54c8fa/db/forward_iterator.cc#L269
• https://github.com/facebook/rocksdb/blob/324453e5790f52557d02abf64a2dcadbaa54c8fa/db/db_impl/db_impl_files.cc#L707

Seems this would making purge to background? Just take a quick glance seems this doesn't have some risk?

Also should we make this a flag? If this has bug, can we quickly turn it off?

[mapleFU]

Seems that speedb also having this. Let's checkin this if other doesn't have any negative views

[wanghenshui]

• https://github.com/facebook/rocksdb/blob/324453e5790f52557d02abf64a2dcadbaa54c8fa/db/forward_iterator.cc#L269

  • https://github.com/facebook/rocksdb/blob/324453e5790f52557d02abf64a2dcadbaa54c8fa/db/db_impl/db_impl_files.cc#L707

Seems this would making purge to background? Just take a quick glance seems this doesn't have some risk?

yes

Also should we make this a flag? If this has bug, can we quickly turn it off?

avoid_unnecessary_blocking_io is global option, could not turn off online

add avoid_unnecessary_blocking_io is false as default

if user don't care iterating with io purge, just dont change it
if user iterates a lot, change config to turn it on

[mapleFU]

avoid_unnecessary_blocking_io is global option, could not turn off online

Just a non-dynamic flag? 🤔 I think it doesn't have any risks except the implement has bug, so set it hardcode true is also ok

[wanghenshui]

avoid_unnecessary_blocking_io is global option, could not turn off online

Just a non-dynamic flag? 🤔 I think it doesn't have any risks except the implement has bug, so set it hardcode true is also ok

flag added. let user know this option and make choice

[git-hulk]

I prefer enabling this option by default since it may reduce the iterator latency. And it'd be better to hide this configuration in kvrocks.conf, because I think users should not care about this configuration.

[mapleFU]

Agree, we can default enable it. But let user know we can disable it if it has bug?

[git-hulk]

Agree, we can default enable it. But let user know we can disable it if it has bug?

My thought is to hide this option in kvrocks.conf if we don't expect users to change this unless any users ask about this. What do you think?

[PragmaTwice]

My thought is to hide this option in kvrocks.conf if we don't expect users to change this unless any users ask about this. What do you think?

Hiding a configuration option in kvrocks.conf can make maintenance more difficult. I believe we should have a document that lists all hidden options.

Simply hiding it without any explanation or documentation does not seem acceptable.

[git-hulk]

My thought is to hide this option in kvrocks.conf if we don't expect users to change this unless any users ask about this. What do you think?

Hiding a configuration option in kvrocks.conf can make maintenance more difficult. I believe we should have a document that lists all hidden options.

Simply hiding it without any explanation or documentation does not seem acceptable.

My point is may overload the configuration file as time goes on. So I'm wondering if we should export every option to users, or only export to them what they should care about. It's good for me to leave as it is.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[mapleFU]

My thought is to hide this option in kvrocks.conf if we don't expect users to change this unless any users ask about this. What do you think?

LGTM