feat: Implement persistence#14
Conversation
…17 and min macosx version to 10.13
- Migrate from in-memory Table to RocksDB-backed Table2 - Implement all core data operations (MutateRow, ReadRows, CheckAndMutateRow, ReadModifyWriteRow) - Add Storage transaction API with ACID guarantees - Solve RocksDB TransactionDB column family creation constraints - Implement dynamic CF management with DB reopen mechanism - Add protobuf serialization for table metadata - Update server endpoints to use persistent Table2 - Clean up debug statements and add TODO markers for future work - Update README with implementation details and completion status - All 9/9 tests passing (8 legacy + 1 new server_test)
refactor: general code refactor
* Document Bazel compilation cache (#7) * bazel: configure clang-tidy (Unoperate#8) * bazel: configure compile_commands.json generator (#6) * feat: Implement GC for Column Families (#3) Co-authored-by: Adam Czajkowski <prawilny@unoperate.com> * docs: unify README style, deduplicate development section (Unoperate#10) * fix: remove unused import --------- Co-authored-by: Adam Czajkowski <prawilny@unoperate.com> Co-authored-by: Brian Gitonga Marete <bgm@unoperate.com>
|
Recent changes:
Should be ready for review. |
styczynski
left a comment
styczynski
left a comment
There was a problem hiding this comment.
Changes: Left self-review comments with questions for reviewers and clarifications about implementation choices.
| @@ -0,0 +1,48 @@ | |||
| { | |||
There was a problem hiding this comment.
review: I think we should leave this file as it's useful for local development.
Querstion to reviewer: how to proceed?
| #include <google/bigtable/v2/data.pb.h> | ||
| #include <google/protobuf/duration.pb.h> | ||
| #include <google/protobuf/repeated_field.h> | ||
| #include <google/protobuf/stubs/mutex.h> |
There was a problem hiding this comment.
review: Is this required? This header seems to be unused in this context and failed to build on my machine.
Querstion to reviewer: Is it ok?
| This means that we need to do some suboptimal stuff in `storage.h`. To stream rows we need to construct iterator. As we group the data we load one row at a time - see `RocksDBStorageRowTX::LoadRow()`. When it happens we | ||
| need to map protobuf into C++ map to get correct ordering. I think this is avoidable somehow, but that kind of performance issue isn't our highest-priority concern right now. | ||
|
|
||
| ### CustomStorage: implementing custom storage interface |
There was a problem hiding this comment.
review: we can provide no-op minimal Storage interface implementation if it's useful
Querstion to reviewer: do you want us to provide minimal no-op implementation as an example of extension for external user?
| # disables C++14 features, even if the compilers defaults to C++ >= 14 | ||
| build:linux --cxxopt=-std=c++14 | ||
| build:macos --cxxopt=-std=c++14 | ||
| build:linux --cxxopt=-std=c++17 |
There was a problem hiding this comment.
review: Our code uses C++17, so it might be necessary to update comment here / and probably README
Querstion to reviewer: should we update README or anything else? Or not?
|
|
||
| /** | ||
| * This is very similar to FilteredTableStream. | ||
| * We didn't want to move all the files to persist/ directory, because that would make diff |
There was a problem hiding this comment.
review: Question for reviewer here how to proceed. This can be done via templatization of original class in table.h.
Querstion to reviewer: Can we proceed and mark it as TODO for someone in the future?
| * (update schema, modify column families, drop row range) against persisted | ||
| * storage. | ||
| */ | ||
| class PersistedTable { |
There was a problem hiding this comment.
review: This is probably misleading name now. We have MemoryStorage so this documentation is no longer true. PersistedTable is just next version of table interface, more generic successor to table.h that can accept any Storage instance.
Querstion to reviewer: should we rename?
| * Storage::Tables() guarantees: | ||
| * - Random const access iterator (specifically std continous range) | ||
| * - Provides snapshot of tables when call to Tables() happened, even if mutations happen in meantime | ||
| */ |
There was a problem hiding this comment.
review: We can probably just remove it entirely and use vector<tuple> directly. It was previosuly used as a utility to hold abstract iterator for abstract Storage (to simplify polymorphism and not use templated Storage class). After refinement I came to conclusion that this interface became thin wrapper around vector.
What probably should be done is:
- Remove this file entirely
- Return vector directly in
Tables()method - We can add method
TablesNames()to storage that will extract names viatransform
Question to reviewer: how to proceed?
Part of RocksDB persistence solution
doc: merge final README + diagram changes
3 participants
This change is
About
This PR adds persistence support to the Bigtable emulator.
AI summary
Short AI-generated summary of changes based on README:
The main implementation lives in
persist/storage.h, which defines an abstractStorageinterface and row-transaction model.PersistedTableprovides the table interface that can be backed by any implementation of this interface. Two backends are provided: MemoryStorage (persist/memory/)—a thin wrapper around the existing in-memory class hierarchy with no persistence—and RocksDBStorage (persist/rocksdb/), which uses RocksDBTransactionDBfor ACID guarantees. Table metadata and configuration use protobufs inpersist/proto/storage.proto. The patch avoids large changes to top-level files while preserving existing behavior and enabling other storage backends in the future.Contributors
PersistentTableimplementationChanges
storage.h, which can be used to implement other backends in the future if needed.persist/memoryaround the existing in-memory class hierarchy so that all existing functionality is preserved.table.h. One such utility isStorageFilteredTableStream. You can removeFilteredTableStreamfromtable.hif you deem it necessary.Future work (from README)
The following items are left for future improvement:
gc_rule. SeeTODOin code.Testing
Tests are provided at several levels:
persist/integration/read_test.ccpersist/memory/storage_test.ccStorageinterface: CreateTable, row transactions, SetCell, DeleteRowColumn, DeleteRowFromColumnFamily, etc.cluster_test.ccserver_test.ccWe also tested manually using CBT to create and read tables and confirmed that data persists across emulator restarts.
Data model