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

fix: treat "sinter sunion sdiff" as wrtie commands #2364

Merged
merged 1 commit into from
Jan 31, 2024
Merged

fix: treat "sinter sunion sdiff" as wrtie commands #2364

merged 1 commit into from
Jan 31, 2024

Conversation

Mixficsol
Copy link
Collaborator

@Mixficsol Mixficsol commented Jan 31, 2024

#2363

Floyd 采用多 RocksDB 实例,每个 Key 可能 Hash 到不同的 RocksDB 实例,与 BlackWidow 不同的是 Floyd 下的 Key 不再以数据类型去区分 RocksDB, 即一个 RocksDB 下可能会存在不同类型的 Key,那么对于 Redis 命令中如:SInter 这种操作多个 Key 的情况下,需要重新设计下确保操作的一致性,对 BlackWidow 来说对于 Sinter 这种命令,由于所有的 Set 类型都在同一个 RocksDB 下,所以对于 SInter 这种读命令不需要进行上 key 锁,只需要在相应的 RocksDB 打一个快照然后进行操作即可,但是在 Floyd 下,多个 Key 可能存在多个 RocksDB 下,对于这种问题我们需要一种解决方式。

image

经过讨论,我们总结出了三种方案解决:

  1. 我们将 sinter 这种命令由原来的读命令设置为写命令,在 CMD 层面上 Key 锁,这样就能完美的解决 Floyd 中的多 key 问题,缺点是 inster 这种本来是读命令不需要进行上锁操作的会变为写命令,但是能保证数据的一致性
  2. 对于 sinter 这种操作多 key 的命令,我们在 Cmd 层上写锁,然后同时在 CMD 层打好每个 Key 对应 Hash 到的 RocksDB 实例的快照,然后以入参的方式从 CMD 层一直传到 Redis 层,这样上锁的粒度会比较小,但是代码的改动量会特别大,会对 CMDStorage , Redis 这三层的接口都需要进行修改,同时还需要对 Floyd 的测试也进行修改
  3. Storage 层上 Key 锁,和上面的 CMD 层上 Key 锁一样,少了 CMD 层对 Storage 层的 snapshot 入参,但是需要在 Storage 中所有的写命令接口中全部上 Key锁,这样的实现方式在 Redis, CMDStorage 层都加上了 Key 锁,感觉代码冗余

总结

这个 PR 中我们采用了第一种解决方案,这也是改动量最小,并且能解决多 Key 操作时的一致性问题

@github-actions github-actions bot added the ☢️ Bug Something isn't working label Jan 31, 2024
@AlexStocks AlexStocks merged commit 2f96952 into OpenAtomFoundation:floyd Jan 31, 2024
4 checks passed
@AlexStocks AlexStocks changed the title fix: floyd keys fix: treat "sinter sunion sdiff" as wrtie commands Jan 31, 2024
@Mixficsol Mixficsol deleted the floyd_keys branch January 31, 2024 13:52
wangshao1 pushed a commit to wangshao1/pika that referenced this pull request Feb 19, 2024
AlexStocks pushed a commit that referenced this pull request Feb 23, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
* feat: add floyd (#2347)

* floyd: refactor storage to support multi rocksdb instance && todis key format

---------

Co-authored-by: wangshaoyi <[email protected]>

* fix floyd keys (#2364)

Co-authored-by: wuxianrong <[email protected]>

* floyd rebase unstable branch

* fix by review comments

---------

Co-authored-by: wangshaoyi <[email protected]>
Co-authored-by: Mixficsol <[email protected]>
Co-authored-by: wuxianrong <[email protected]>
bigdaronlee163 pushed a commit to bigdaronlee163/pika that referenced this pull request Jun 8, 2024
* feat: add floyd (OpenAtomFoundation#2347)

* floyd: refactor storage to support multi rocksdb instance && todis key format

---------

Co-authored-by: wangshaoyi <[email protected]>

* fix floyd keys (OpenAtomFoundation#2364)

Co-authored-by: wuxianrong <[email protected]>

* floyd rebase unstable branch

* fix by review comments

---------

Co-authored-by: wangshaoyi <[email protected]>
Co-authored-by: Mixficsol <[email protected]>
Co-authored-by: wuxianrong <[email protected]>
cheniujh pushed a commit to cheniujh/pika that referenced this pull request Sep 24, 2024
* feat: add floyd (OpenAtomFoundation#2347)

* floyd: refactor storage to support multi rocksdb instance && todis key format

---------

Co-authored-by: wangshaoyi <[email protected]>

* fix floyd keys (OpenAtomFoundation#2364)

Co-authored-by: wuxianrong <[email protected]>

* floyd rebase unstable branch

* fix by review comments

---------

Co-authored-by: wangshaoyi <[email protected]>
Co-authored-by: Mixficsol <[email protected]>
Co-authored-by: wuxianrong <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
☢️ Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants