kvrocks icon indicating copy to clipboard operation
kvrocks copied to clipboard

perf: Set async_io default value to yes

Open xiaobiaozhao opened this issue 1 year ago • 6 comments

async_io had ready, I think kvrocks should use async_io as default value.

xiaobiaozhao avatar May 11 '24 13:05 xiaobiaozhao

@xiaobiaozhao You need to turn the default value of rocksdb.async_io to true as well. Another quick question: is the rocksdb community marking this feature as stable?

git-hulk avatar May 13 '24 08:05 git-hulk

The feature has been unlabeled for experimentation, but rocksdb has not enabled it by default.

xiaobiaozhao avatar May 16 '24 00:05 xiaobiaozhao

This is LGTM. Seems for Scan it depends on advanced fs api and could decrease latency on some disk. I don't know on some fast NVMe disk, would this helps a lot? Also maybe it doesn't works well on some Linux kernel without io_uring?

Anyway this is ok

Do you mean on older kernels? I just test on ubuntu 20/22/24 , it works well.

xiaobiaozhao avatar May 26 '24 10:05 xiaobiaozhao

Do you mean on older kernels? I just test on ubuntu 20/22/24 , it works well.

On legacy os it uses "Read" rather than ReadAsync api

mapleFU avatar May 27 '24 02:05 mapleFU

I've check the code of async_io:

document:

  // If async_io is enabled, RocksDB will prefetch some of data asynchronously.
  // RocksDB apply it if reads are sequential and its internal automatic
  // prefetching.
  bool async_io = false;

For rocksdb, related macro is

  • WITH_LIBURING ROCKSDB_IOURING_PRESENT: io_uring related
  • USE_COROUTINES USE_FOLLY: MultiGet related, it will using coroutine to schedule issued task

I didn't check do we enabled this.

For fs io, a code below would be triggered:

inline bool CheckFSFeatureSupport(FileSystem* fs, FSSupportedOps feat) {
  int64_t supported_ops = 0;
  fs->SupportedOps(supported_ops);
  if (supported_ops & (1ULL << feat)) {
    return true;
  }
  return false;
}

If fs doesn't supports async (.i.e no io_uring ) I guess this might making sense in MultiGet.

The ForwardIterator will also check the async feature is support.

I also checked the code for iterator, BlockPrefetcher might be issued io for this, so, without io_uring, scan might be optimized(with larger io prefetch-depth)? I didn't run the test, maybe it work ( or maybe not )

mapleFU avatar May 27 '24 14:05 mapleFU

Yeah we need to check the implementation for its behavior.

In docker image, seems we don't install liburing at all. Not sure if the option has effect.

PragmaTwice avatar May 31 '24 14:05 PragmaTwice

Yeah we need to check the implementation for its behavior.

In docker image, seems we don't install liburing at all. Not sure if the option has effect.

here is a test report

https://github.com/apache/kvrocks/pull/1215

xiaobiaozhao avatar Jun 01 '24 15:06 xiaobiaozhao

Quality Gate Failed Quality Gate failed

Failed conditions
11.9% Coverage on New Code (required ≥ 50%)

See analysis details on SonarCloud

sonarqubecloud[bot] avatar Jun 04 '24 03:06 sonarqubecloud[bot]

## 8.1.0 (03/18/2023)
### Behavior changes
* Compaction output file cutting logic now considers range tombstone start keys. For example, SST partitioner now may receive ParitionRequest for range tombstone start keys.
* If the async_io ReadOption is specified for MultiGet or NewIterator on a platform that doesn't support IO uring, the option is ignored and synchronous IO is used.

PragmaTwice avatar Jun 04 '24 05:06 PragmaTwice