chore: add assertion to ensure monotonically decreasing user key epoch #14488

hzxa21 · 2024-01-10T11:02:25Z

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

This PR adds assertion in the following places to ensure monotonically decreasing user key epoch:

user key iterator
compaction iterator
imm merge iterator

Checklist

I have written necessary rustdoc comments
I have added necessary unit tests and integration tests
I have added test labels as necessary. See details.
I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
All checks passed in ./risedev check (or alias, ./risedev c)
My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)

My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details)

Documentation

My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

Release note

If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.

wcy-fdu · 2024-01-10T11:30:33Z

The check does not pass in e2e test.

thread 'rw-streaming' panicked at src/storage/src/hummock/iterator/forward_user.rs:157:21:
assertion failed: last_seen_epoch_with_gap > full_key.epoch_with_gap
   0: rust_begin_unwind
             at /rustc/e4c626dd9a17a23270bf8e7158e59cf2b9c04840/library/std/src/panicking.rs:645:5
   1: core::panicking::panic_fmt
             at /rustc/e4c626dd9a17a23270bf8e7158e59cf2b9c04840/library/core/src/panicking.rs:72:14
   2: core::panicking::panic
             at /rustc/e4c626dd9a17a23270bf8e7158e59cf2b9c04840/library/core/src/panicking.rs:144:5
   3: {async_fn#0}<risingwave_storage::hummock::iterator::skip_watermark::SkipWatermarkIterator<risingwave_storage::hummock::iterator::merge_inner::MergeIteratorInner<risingwave_storage::hummock::iterator::HummockIteratorUnion<risingwave_storage::hummock::iterator::Forward, risingwave_storage::hummock::iterator::merge_inner::MergeIteratorInner<risingwave_storage::hummock::iterator::HummockIteratorUnion<risingwave_storage::hummock::iterator::Forward, risingwave_storage::hummock::shared_buffer::shared_buffer_batch::SharedBufferBatchIterator<risingwave_storage::hummock::iterator::Forward>, risingwave_storage::hummock::sstable::forward_sstable_iterator::SstableIterator, risingwave_storage::hummock::iterator::PhantomHummockIterator<risingwave_storage::hummock::iterator::Forward>, risingwave_storage::hummock::iterator::PhantomHummockIterator<risingwave_storage::hummock::iterator::Forward>>, usize>, risingwave_storage::hummock::sstable::forward_sstable_iterator::SstableIterator, risingwave_storage::hummock::iterator::concat_inner::ConcatIteratorInner<risingwave_storage::hummock::sstable::forward_sstable_iterator::SstableIterator>, risingwave_storage::hummock::iterator::FromRustIterator<risingwave_storage::mem_table::MemTableIteratorBuilder>>, ()>>>
             at ./src/storage/src/hummock/iterator/forward_user.rs:157:21
   4: {async_fn#0}
             at ./src/storage/src/hummock/store/local_hummock_storage.rs:600:25
   5: {coroutine#0}<risingwave_storage::hummock::store::local_hummock_storage::HummockStorageIteratorInner>
             at ./src/storage/src/store.rs:58:40
   6: poll_next<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::hummock::store::local_hummock_storage::HummockStorageIteratorInner>, (risingwave_hummock_sdk::key::FullKey<bytes::bytes::Bytes>, bytes::bytes::Bytes), risingwave_storage::error::StorageError>
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-async-stream-0.2.9/src/lib.rs:506:33
   7: poll_next<&mut futures_async_stream::try_stream::GenTryStream<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::hummock::store::local_hummock_storage::HummockStorageIteratorInner>>>
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-core-0.3.28/src/stream.rs:120:9
   8: <S as futures_core::stream::TryStream>::try_poll_next
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-core-0.3.28/src/stream.rs:196:9
   9: try_poll_next_unpin<core::pin::Pin<&mut futures_async_stream::try_stream::GenTryStream<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::hummock::store::local_hummock_storage::HummockStorageIteratorInner>>>>
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.28/src/stream/try_stream/mod.rs:1068:9
  10: <futures_util::stream::try_stream::try_next::TryNext<St> as core::future::future::Future>::poll
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.28/src/stream/try_stream/try_next.rs:32:9
  11: {coroutine#0}<futures_async_stream::try_stream::GenTryStream<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::hummock::store::local_hummock_storage::HummockStorageIteratorInner>>, futures_async_stream::try_stream::GenTryStream<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::memory::RangeKvStateStoreIter<risingwave_storage::memory::sled::SledRangeKv>>>>
             at ./src/storage/src/store_impl.rs:306:44
  12: poll_next<risingwave_storage::store_impl::verify::verify_stream::{coroutine_env#0}<futures_async_stream::try_stream::GenTryStream<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::hummock::store::local_hummock_storage::HummockStorageIteratorInner>>, futures_async_stream::try_stream::GenTryStream<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::memory::RangeKvStateStoreIter<risingwave_storage::memory::sled::SledRangeKv>>>>, (risingwave_hummock_sdk::key::FullKey<bytes::bytes::Bytes>, bytes::bytes::Bytes), risingwave_storage::error::StorageError>
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-async-stream-0.2.9/src/lib.rs:506:33
  13: poll_next<alloc::boxed::Box<(dyn futures_core::stream::Stream<Item=core::result::Result<(risingwave_hummock_sdk::key::FullKey<bytes::bytes::Bytes>, bytes::bytes::Bytes), risingwave_storage::error::StorageError>> + core::marker::Send), alloc::alloc::Global>>
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-core-0.3.28/src/stream.rs:120:9

hzxa21 · 2024-01-10T13:14:39Z

The check does not pass in e2e test.

thread 'rw-streaming' panicked at src/storage/src/hummock/iterator/forward_user.rs:157:21:
assertion failed: last_seen_epoch_with_gap > full_key.epoch_with_gap
   0: rust_begin_unwind
             at /rustc/e4c626dd9a17a23270bf8e7158e59cf2b9c04840/library/std/src/panicking.rs:645:5
   1: core::panicking::panic_fmt
             at /rustc/e4c626dd9a17a23270bf8e7158e59cf2b9c04840/library/core/src/panicking.rs:72:14
   2: core::panicking::panic
             at /rustc/e4c626dd9a17a23270bf8e7158e59cf2b9c04840/library/core/src/panicking.rs:144:5
   3: {async_fn#0}<risingwave_storage::hummock::iterator::skip_watermark::SkipWatermarkIterator<risingwave_storage::hummock::iterator::merge_inner::MergeIteratorInner<risingwave_storage::hummock::iterator::HummockIteratorUnion<risingwave_storage::hummock::iterator::Forward, risingwave_storage::hummock::iterator::merge_inner::MergeIteratorInner<risingwave_storage::hummock::iterator::HummockIteratorUnion<risingwave_storage::hummock::iterator::Forward, risingwave_storage::hummock::shared_buffer::shared_buffer_batch::SharedBufferBatchIterator<risingwave_storage::hummock::iterator::Forward>, risingwave_storage::hummock::sstable::forward_sstable_iterator::SstableIterator, risingwave_storage::hummock::iterator::PhantomHummockIterator<risingwave_storage::hummock::iterator::Forward>, risingwave_storage::hummock::iterator::PhantomHummockIterator<risingwave_storage::hummock::iterator::Forward>>, usize>, risingwave_storage::hummock::sstable::forward_sstable_iterator::SstableIterator, risingwave_storage::hummock::iterator::concat_inner::ConcatIteratorInner<risingwave_storage::hummock::sstable::forward_sstable_iterator::SstableIterator>, risingwave_storage::hummock::iterator::FromRustIterator<risingwave_storage::mem_table::MemTableIteratorBuilder>>, ()>>>
             at ./src/storage/src/hummock/iterator/forward_user.rs:157:21
   4: {async_fn#0}
             at ./src/storage/src/hummock/store/local_hummock_storage.rs:600:25
   5: {coroutine#0}<risingwave_storage::hummock::store::local_hummock_storage::HummockStorageIteratorInner>
             at ./src/storage/src/store.rs:58:40
   6: poll_next<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::hummock::store::local_hummock_storage::HummockStorageIteratorInner>, (risingwave_hummock_sdk::key::FullKey<bytes::bytes::Bytes>, bytes::bytes::Bytes), risingwave_storage::error::StorageError>
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-async-stream-0.2.9/src/lib.rs:506:33
   7: poll_next<&mut futures_async_stream::try_stream::GenTryStream<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::hummock::store::local_hummock_storage::HummockStorageIteratorInner>>>
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-core-0.3.28/src/stream.rs:120:9
   8: <S as futures_core::stream::TryStream>::try_poll_next
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-core-0.3.28/src/stream.rs:196:9
   9: try_poll_next_unpin<core::pin::Pin<&mut futures_async_stream::try_stream::GenTryStream<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::hummock::store::local_hummock_storage::HummockStorageIteratorInner>>>>
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.28/src/stream/try_stream/mod.rs:1068:9
  10: <futures_util::stream::try_stream::try_next::TryNext<St> as core::future::future::Future>::poll
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.28/src/stream/try_stream/try_next.rs:32:9
  11: {coroutine#0}<futures_async_stream::try_stream::GenTryStream<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::hummock::store::local_hummock_storage::HummockStorageIteratorInner>>, futures_async_stream::try_stream::GenTryStream<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::memory::RangeKvStateStoreIter<risingwave_storage::memory::sled::SledRangeKv>>>>
             at ./src/storage/src/store_impl.rs:306:44
  12: poll_next<risingwave_storage::store_impl::verify::verify_stream::{coroutine_env#0}<futures_async_stream::try_stream::GenTryStream<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::hummock::store::local_hummock_storage::HummockStorageIteratorInner>>, futures_async_stream::try_stream::GenTryStream<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::memory::RangeKvStateStoreIter<risingwave_storage::memory::sled::SledRangeKv>>>>, (risingwave_hummock_sdk::key::FullKey<bytes::bytes::Bytes>, bytes::bytes::Bytes), risingwave_storage::error::StorageError>
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-async-stream-0.2.9/src/lib.rs:506:33
  13: poll_next<alloc::boxed::Box<(dyn futures_core::stream::Stream<Item=core::result::Result<(risingwave_hummock_sdk::key::FullKey<bytes::bytes::Bytes>, bytes::bytes::Bytes), risingwave_storage::error::StorageError>> + core::marker::Send), alloc::alloc::Global>>
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-core-0.3.28/src/stream.rs:120:9

Just found out the cause. Will fix.

hzxa21 · 2024-01-17T15:10:33Z

The check does not pass in e2e test.

thread 'rw-streaming' panicked at src/storage/src/hummock/iterator/forward_user.rs:157:21:
assertion failed: last_seen_epoch_with_gap > full_key.epoch_with_gap
   0: rust_begin_unwind
             at /rustc/e4c626dd9a17a23270bf8e7158e59cf2b9c04840/library/std/src/panicking.rs:645:5
   1: core::panicking::panic_fmt
             at /rustc/e4c626dd9a17a23270bf8e7158e59cf2b9c04840/library/core/src/panicking.rs:72:14
   2: core::panicking::panic
             at /rustc/e4c626dd9a17a23270bf8e7158e59cf2b9c04840/library/core/src/panicking.rs:144:5
   3: {async_fn#0}<risingwave_storage::hummock::iterator::skip_watermark::SkipWatermarkIterator<risingwave_storage::hummock::iterator::merge_inner::MergeIteratorInner<risingwave_storage::hummock::iterator::HummockIteratorUnion<risingwave_storage::hummock::iterator::Forward, risingwave_storage::hummock::iterator::merge_inner::MergeIteratorInner<risingwave_storage::hummock::iterator::HummockIteratorUnion<risingwave_storage::hummock::iterator::Forward, risingwave_storage::hummock::shared_buffer::shared_buffer_batch::SharedBufferBatchIterator<risingwave_storage::hummock::iterator::Forward>, risingwave_storage::hummock::sstable::forward_sstable_iterator::SstableIterator, risingwave_storage::hummock::iterator::PhantomHummockIterator<risingwave_storage::hummock::iterator::Forward>, risingwave_storage::hummock::iterator::PhantomHummockIterator<risingwave_storage::hummock::iterator::Forward>>, usize>, risingwave_storage::hummock::sstable::forward_sstable_iterator::SstableIterator, risingwave_storage::hummock::iterator::concat_inner::ConcatIteratorInner<risingwave_storage::hummock::sstable::forward_sstable_iterator::SstableIterator>, risingwave_storage::hummock::iterator::FromRustIterator<risingwave_storage::mem_table::MemTableIteratorBuilder>>, ()>>>
             at ./src/storage/src/hummock/iterator/forward_user.rs:157:21
   4: {async_fn#0}
             at ./src/storage/src/hummock/store/local_hummock_storage.rs:600:25
   5: {coroutine#0}<risingwave_storage::hummock::store::local_hummock_storage::HummockStorageIteratorInner>
             at ./src/storage/src/store.rs:58:40
   6: poll_next<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::hummock::store::local_hummock_storage::HummockStorageIteratorInner>, (risingwave_hummock_sdk::key::FullKey<bytes::bytes::Bytes>, bytes::bytes::Bytes), risingwave_storage::error::StorageError>
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-async-stream-0.2.9/src/lib.rs:506:33
   7: poll_next<&mut futures_async_stream::try_stream::GenTryStream<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::hummock::store::local_hummock_storage::HummockStorageIteratorInner>>>
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-core-0.3.28/src/stream.rs:120:9
   8: <S as futures_core::stream::TryStream>::try_poll_next
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-core-0.3.28/src/stream.rs:196:9
   9: try_poll_next_unpin<core::pin::Pin<&mut futures_async_stream::try_stream::GenTryStream<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::hummock::store::local_hummock_storage::HummockStorageIteratorInner>>>>
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.28/src/stream/try_stream/mod.rs:1068:9
  10: <futures_util::stream::try_stream::try_next::TryNext<St> as core::future::future::Future>::poll
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.28/src/stream/try_stream/try_next.rs:32:9
  11: {coroutine#0}<futures_async_stream::try_stream::GenTryStream<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::hummock::store::local_hummock_storage::HummockStorageIteratorInner>>, futures_async_stream::try_stream::GenTryStream<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::memory::RangeKvStateStoreIter<risingwave_storage::memory::sled::SledRangeKv>>>>
             at ./src/storage/src/store_impl.rs:306:44
  12: poll_next<risingwave_storage::store_impl::verify::verify_stream::{coroutine_env#0}<futures_async_stream::try_stream::GenTryStream<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::hummock::store::local_hummock_storage::HummockStorageIteratorInner>>, futures_async_stream::try_stream::GenTryStream<risingwave_storage::store::into_stream_inner::{coroutine_env#0}<risingwave_storage::memory::RangeKvStateStoreIter<risingwave_storage::memory::sled::SledRangeKv>>>>, (risingwave_hummock_sdk::key::FullKey<bytes::bytes::Bytes>, bytes::bytes::Bytes), risingwave_storage::error::StorageError>
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-async-stream-0.2.9/src/lib.rs:506:33
  13: poll_next<alloc::boxed::Box<(dyn futures_core::stream::Stream<Item=core::result::Result<(risingwave_hummock_sdk::key::FullKey<bytes::bytes::Bytes>, bytes::bytes::Bytes), risingwave_storage::error::StorageError>> + core::marker::Send), alloc::alloc::Global>>
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-core-0.3.28/src/stream.rs:120:9

Just found out the cause. Will fix.

Fixed by #14494

hzxa21 · 2024-01-17T15:11:18Z

Introduced FullKeyTracker to unify the checking logic in different places.

zwang28

LGTM

wenym1

Rest LGTM

wenym1 · 2024-01-19T11:04:09Z

src/storage/src/hummock/compactor/shared_buffer_compact.rs

@@ -427,13 +428,16 @@ pub async fn merge_imms_in_memory(
    }

    let mut merged_payload: Vec<SharedBufferVersionedEntry> = Vec::new();
-    let mut pivot = items
+    let first_item_key = items


maybe off topic: can we assert that items is non-empty instead of creating an implicit default value?

I planned to do that before but I later realized it is possible that the items are empty when imm only contains range tombstone.

However, I am planning to disallow imm merge when there are range tombstones in imm, which can simplify the logic a lot.

src/storage/hummock_sdk/src/key.rs

src/storage/src/hummock/compactor/shared_buffer_compact.rs

hzxa21 requested review from wenym1, wcy-fdu and zwang28 January 10, 2024 11:02

github-actions bot added the type/chore label Jan 10, 2024

hzxa21 force-pushed the patrick/user-key-epoch-assert branch from 17a7e5d to 7de0046 Compare January 17, 2024 15:04

hzxa21 mentioned this pull request Jan 18, 2024

bug: storage table read can read the same staging SST multiple times #14659

Closed

zwang28 approved these changes Jan 19, 2024

View reviewed changes

wenym1 approved these changes Jan 19, 2024

View reviewed changes

hzxa21 added 7 commits January 23, 2024 20:39

chore: add assertion to ensure monotonically decreasing user key epoch

23fa757

rename

5a993ce

fix merge imm test

15ca52f

fix ut

62ea204

fix check

90961bf

fix check

2b7d4f1

address comments

9781d5f

hzxa21 force-pushed the patrick/user-key-epoch-assert branch from 91dd648 to 9781d5f Compare January 23, 2024 13:00

hzxa21 added 4 commits January 24, 2024 13:34

fix check

2fc4895

fix check

cfb0faa

fix doc test

77a7be3

fix issue due to enable_test_epoch

d2dfda2

hzxa21 added this pull request to the merge queue Jan 24, 2024

Merged via the queue into main with commit c67e4e4 Jan 24, 2024
26 of 27 checks passed

hzxa21 deleted the patrick/user-key-epoch-assert branch January 24, 2024 10:16

cyliu0 mentioned this pull request Jan 31, 2024

nightly-20240126 deleting data from backfill table stuck #14886

Closed

zwang28 mentioned this pull request Feb 17, 2024

bug(storage): barrier read may return duplicate full key #15099

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: add assertion to ensure monotonically decreasing user key epoch #14488

chore: add assertion to ensure monotonically decreasing user key epoch #14488

hzxa21 commented Jan 10, 2024

wcy-fdu commented Jan 10, 2024

hzxa21 commented Jan 10, 2024

hzxa21 commented Jan 17, 2024

hzxa21 commented Jan 17, 2024

zwang28 left a comment

wenym1 left a comment

wenym1 Jan 19, 2024

hzxa21 Jan 23, 2024

chore: add assertion to ensure monotonically decreasing user key epoch #14488

chore: add assertion to ensure monotonically decreasing user key epoch #14488

Conversation

hzxa21 commented Jan 10, 2024

What's changed and what's your intention?

Checklist

Documentation

Release note

wcy-fdu commented Jan 10, 2024

hzxa21 commented Jan 10, 2024

hzxa21 commented Jan 17, 2024

hzxa21 commented Jan 17, 2024

zwang28 left a comment

Choose a reason for hiding this comment

wenym1 left a comment

Choose a reason for hiding this comment

wenym1 Jan 19, 2024

Choose a reason for hiding this comment

hzxa21 Jan 23, 2024

Choose a reason for hiding this comment