Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KAFKA-14412: Decouple RocksDB access from CF #15105

Merged
merged 1 commit into from
Jan 4, 2024

Conversation

nicktelford
Copy link
Contributor

To support future use-cases that use different strategies for accessing RocksDB, we need to de-couple the RocksDB access strategy from the Column Family access strategy.

To do this, we now have two separate accessors:

  • DBAccessor: dictates how we access RocksDB. Currently only one strategy is supported: DirectDBAccessor, which access RocksDB directly, via the RocksDB class for all operations. In the future, a BatchedDBAccessor will be added, which enables transactions via WriteBatch.
  • ColumnFamilyAccessor: maps StateStore operations to operations on one or more column families. This is a rename of the old RocksDBDBAccessor.

@nicktelford
Copy link
Contributor Author

@cadonna @mjsax @ableegoldman @lucasbru @wcarlson5 @bbejeck @vvcephei @guozhangwang

This is part of KIP-892, and has been broken out into a separate PR to reduce the review burden on the main KIP-892 implementation, since it can be merged independently.

There are no tests, because there are no behavioural changes, just a refactoring. The existing test suite should ensure no regressions.

@nicktelford nicktelford force-pushed the rocksdb-refactor-accessor-2 branch 2 times, most recently from 4e94445 to fdd9171 Compare January 2, 2024 11:58
Copy link
Member

@lucasbru lucasbru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM!

@lucasbru
Copy link
Member

lucasbru commented Jan 3, 2024

@nicktelford Seems like all build jobs timed out. Could you take a look?

@nicktelford
Copy link
Contributor Author

@lucasbru I think this is just the CI causing trouble again. The build passes locally. Is there a way to rerun the build without pushing more commits?

@lucasbru
Copy link
Member

lucasbru commented Jan 3, 2024

@nicktelford I'll rerun it. That being said, the last jobs on trunk have all finished within 3-5 hours, so this must be caused by either infrastructure or the code in this PR.

To support future use-cases that use different strategies for accessing
RocksDB, we need to de-couple the RocksDB access strategy from the
Column Family access strategy.

To do this, we now have two separate accessors:

  * `DBAccessor`: dictates how we access RocksDB. Currently only one
    strategy is supported: `DirectDBAccessor`, which access RocksDB
    directly, via the `RocksDB` class for all operations. In the future, a
    `BatchedDBAccessor` will be added, which enables transactions via
    `WriteBatch`.
  * `ColumnFamilyAccessor`: maps StateStore operations to operations on
    one or more column families. This is a rename of the old
    `RocksDBDBAccessor`.
@nicktelford
Copy link
Contributor Author

@lucasbru OK, my bad. It turns out I did a minor refactoring after I ran the test suite yesterday that was so insignificant I didn't think I needed to run the tests again... Turns out I was wrong 🙈

I've fixed the bug now and the tests pass locally for real now.

@lucasbru lucasbru merged commit 5bc3aa4 into apache:trunk Jan 4, 2024
1 check failed
@nicktelford nicktelford deleted the rocksdb-refactor-accessor-2 branch January 4, 2024 12:19
lucasbru added a commit to lucasbru/kafka that referenced this pull request Jan 10, 2024
nicktelford added a commit to nicktelford/kafka that referenced this pull request Jan 12, 2024
To support future use-cases that use different strategies for accessing
RocksDB, we need to de-couple the RocksDB access strategy from the
Column Family access strategy.

To do this, we now have two separate accessors:

  * `DBAccessor`: dictates how we access RocksDB. Currently only one
    strategy is supported: `DirectDBAccessor`, which access RocksDB
    directly, via the `RocksDB` class for all operations. In the future, a
    `BatchedDBAccessor` will be added, which enables transactions via
    `WriteBatch`.
  * `ColumnFamilyAccessor`: maps StateStore operations to operations on
    one or more column families. This is a rename of the old
    `RocksDBDBAccessor`.

Reviewers: Lucas Brutschy <lbrutschy@confluent.io>
@mjsax mjsax added the streams label Jan 17, 2024
nicktelford added a commit to nicktelford/kafka that referenced this pull request Jan 24, 2024
To support future use-cases that use different strategies for accessing
RocksDB, we need to de-couple the RocksDB access strategy from the
Column Family access strategy.

To do this, we now have two separate accessors:

  * `DBAccessor`: dictates how we access RocksDB. Currently only one
    strategy is supported: `DirectDBAccessor`, which access RocksDB
    directly, via the `RocksDB` class for all operations. In the future, a
    `BatchedDBAccessor` will be added, which enables transactions via
    `WriteBatch`.
  * `ColumnFamilyAccessor`: maps StateStore operations to operations on
    one or more column families. This is a rename of the old
    `RocksDBDBAccessor`.

Reviewers: Lucas Brutschy <lbrutschy@confluent.io>
yyu1993 pushed a commit to yyu1993/kafka that referenced this pull request Feb 15, 2024
To support future use-cases that use different strategies for accessing
RocksDB, we need to de-couple the RocksDB access strategy from the
Column Family access strategy.

To do this, we now have two separate accessors:

  * `DBAccessor`: dictates how we access RocksDB. Currently only one
    strategy is supported: `DirectDBAccessor`, which access RocksDB
    directly, via the `RocksDB` class for all operations. In the future, a
    `BatchedDBAccessor` will be added, which enables transactions via
    `WriteBatch`.
  * `ColumnFamilyAccessor`: maps StateStore operations to operations on
    one or more column families. This is a rename of the old
    `RocksDBDBAccessor`.

Reviewers: Lucas Brutschy <lbrutschy@confluent.io>
AnatolyPopov pushed a commit to aiven/kafka that referenced this pull request Feb 16, 2024
To support future use-cases that use different strategies for accessing
RocksDB, we need to de-couple the RocksDB access strategy from the
Column Family access strategy.

To do this, we now have two separate accessors:

  * `DBAccessor`: dictates how we access RocksDB. Currently only one
    strategy is supported: `DirectDBAccessor`, which access RocksDB
    directly, via the `RocksDB` class for all operations. In the future, a
    `BatchedDBAccessor` will be added, which enables transactions via
    `WriteBatch`.
  * `ColumnFamilyAccessor`: maps StateStore operations to operations on
    one or more column families. This is a rename of the old
    `RocksDBDBAccessor`.

Reviewers: Lucas Brutschy <lbrutschy@confluent.io>
@mjsax mjsax added the kip Requires or implements a KIP label Feb 23, 2024
clolov pushed a commit to clolov/kafka that referenced this pull request Apr 5, 2024
To support future use-cases that use different strategies for accessing
RocksDB, we need to de-couple the RocksDB access strategy from the
Column Family access strategy.

To do this, we now have two separate accessors:

  * `DBAccessor`: dictates how we access RocksDB. Currently only one
    strategy is supported: `DirectDBAccessor`, which access RocksDB
    directly, via the `RocksDB` class for all operations. In the future, a
    `BatchedDBAccessor` will be added, which enables transactions via
    `WriteBatch`.
  * `ColumnFamilyAccessor`: maps StateStore operations to operations on
    one or more column families. This is a rename of the old
    `RocksDBDBAccessor`.

Reviewers: Lucas Brutschy <lbrutschy@confluent.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kip Requires or implements a KIP streams
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants