From e015206dd6e8409d34e79dc353c00c543a6d3a0e Mon Sep 17 00:00:00 2001 From: anand76 Date: Fri, 20 May 2022 12:38:21 -0700 Subject: [PATCH] Fix crash due to MultiGet async IO and direct IO (#10024) Summary: MultiGet with async IO is not officially supported with Posix yet. Avoid a crash by using synchronous MultiRead when direct IO is enabled. Pull Request resolved: https://github.com/facebook/rocksdb/pull/10024 Test Plan: Run db_crashtest.py manually Reviewed By: akankshamahajan15 Differential Revision: D36551053 Pulled By: anand1976 fbshipit-source-id: 72190418fa92dd0397e87825df618b12c9bdecda --- db/table_cache.cc | 1 + db/version_set.cc | 1 + table/block_based/block_based_table_reader.cc | 1 + .../block_based_table_reader_sync_and_async.h | 16 ++++++++++------ 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/db/table_cache.cc b/db/table_cache.cc index e121e66b2..e9ef7acf1 100644 --- a/db/table_cache.cc +++ b/db/table_cache.cc @@ -51,6 +51,7 @@ static void DeleteEntry(const Slice& /*key*/, void* value) { #undef WITHOUT_COROUTINES #define WITH_COROUTINES #include "db/table_cache_sync_and_async.h" +#undef WITH_COROUTINES // clang-format on namespace ROCKSDB_NAMESPACE { diff --git a/db/version_set.cc b/db/version_set.cc index f9bfd66d4..108347745 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -82,6 +82,7 @@ #undef WITHOUT_COROUTINES #define WITH_COROUTINES #include "db/version_set_sync_and_async.h" +#undef WITH_COROUTINES // clang-format on namespace ROCKSDB_NAMESPACE { diff --git a/table/block_based/block_based_table_reader.cc b/table/block_based/block_based_table_reader.cc index 3f3b5c481..50ad9c245 100644 --- a/table/block_based/block_based_table_reader.cc +++ b/table/block_based/block_based_table_reader.cc @@ -96,6 +96,7 @@ CacheAllocationPtr CopyBufferToHeap(MemoryAllocator* allocator, Slice& buf) { #undef WITHOUT_COROUTINES #define WITH_COROUTINES #include "table/block_based/block_based_table_reader_sync_and_async.h" +#undef WITH_COROUTINES // clang-format on namespace ROCKSDB_NAMESPACE { diff --git a/table/block_based/block_based_table_reader_sync_and_async.h b/table/block_based/block_based_table_reader_sync_and_async.h index eaf07f215..2ab9882fc 100644 --- a/table/block_based/block_based_table_reader_sync_and_async.h +++ b/table/block_based/block_based_table_reader_sync_and_async.h @@ -140,12 +140,16 @@ DEFINE_SYNC_AND_ASYNC(void, BlockBasedTable::RetrieveMultipleBlocks) IOOptions opts; IOStatus s = file->PrepareIOOptions(options, opts); if (s.ok()) { -#if defined(WITHOUT_COROUTINES) - s = file->MultiRead(opts, &read_reqs[0], read_reqs.size(), &direct_io_buf, - options.rate_limiter_priority); -#else // WITH_COROUTINES - co_await batch->context()->reader().MultiReadAsync( - file, opts, &read_reqs[0], read_reqs.size(), &direct_io_buf); +#if defined(WITH_COROUTINES) + if (file->use_direct_io()) { +#endif // WITH_COROUTINES + s = file->MultiRead(opts, &read_reqs[0], read_reqs.size(), + &direct_io_buf, options.rate_limiter_priority); +#if defined(WITH_COROUTINES) + } else { + co_await batch->context()->reader().MultiReadAsync( + file, opts, &read_reqs[0], read_reqs.size(), &direct_io_buf); + } #endif // WITH_COROUTINES } if (!s.ok()) {