Skip to content

Add support of stream migration during slot migration process#1197

Merged
PragmaTwice merged 3 commits intoapache:unstablefrom
torwig:slot_migrate_streams
Dec 21, 2022
Merged

Add support of stream migration during slot migration process#1197
PragmaTwice merged 3 commits intoapache:unstablefrom
torwig:slot_migrate_streams

Conversation

@torwig
Copy link
Contributor

@torwig torwig commented Dec 19, 2022

The XSETID command was implemented. This command is used only during stream key migration.
It differs from the same Redis command just in one case: in Redis, you can't apply it to a non-existing stream; in Kvrocks you can because streams are allowed to be empty and there should be a way to migrate an empty stream (with no prior XADD command which can create a stream if it doesn't exist).
Related C++ unit tests were added.

Stream key migration was implemented. The key of the stream type is migrated via a separate function.
Related Go tests were added.

Refactor and tidy cluster-related code (src/cluster folder).

Closes #1087

@git-hulk git-hulk requested review from ShooterIT, caipengbo, git-hulk, jbonofre and tisonkun and removed request for jbonofre December 20, 2022 01:38
bool MigrateSimpleKey(const rocksdb::Slice &key, const Metadata &metadata, const std::string &bytes,
std::string *restore_cmds);
bool MigrateComplexKey(const rocksdb::Slice &key, const Metadata &metadata, std::string *restore_cmds);
bool MigrateStream(const rocksdb::Slice &key, const StreamMetadata &metadata, std::string *restore_cmds);
Copy link
Contributor

@caipengbo caipengbo Dec 21, 2022

Choose a reason for hiding this comment

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

By the way, this type of functions uses bool as its return value. I think it's a more elegant way to return Status, and move the LOG information from these internal functions to Status Msg, and return it to the upper level.

Copy link
Member

Choose a reason for hiding this comment

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

We can refactor them in further PRs : )

@PragmaTwice
Copy link
Member

Thanks all. Merging...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for migrating STREAM in cluster mode

4 participants