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

feat: support blpop/brpop #1548

Merged
merged 57 commits into from
Jul 19, 2023

Conversation

cheniujh
Copy link
Collaborator

@cheniujh cheniujh commented May 23, 2023

新增对blpop、brpop命令的支持。

基本思路:
1.维护了一个map,用于记录key与对应的阻塞客户端,大致如下:
image

  1. 另外加了一个辅助的map,可以避免一些不必要的遍历:
image

这两个map以及一个对应的std::shared_mutex加到了DispatchThread中作为成员变量。

对这两个map的操作:
1.当blpop/brpop无法从目标list中取得元素时,往maps_from_keys_to_conns中对应key的list中新增Node(也往map_from_conns_to_keys插入一个list of keys)。

2.当lpush,rpush,rpoplpush命令执行后,用maps_from_keys_to_conns执行一下find方法,看是否有客户端正在等待当前key(刚刚发生push行为的),如果有,则给往线程池塞一个任务,用于向被阻塞的客户端写响应,以及清理两个map中的对应项。

3.超时扫描:在serverThread的threadmain的epoll里注册定时任务(目前是200ms执行一次),每次扫描整个maps_from_keys_to_conns,查看是否有过期的Node,如果有,回写一个-1(nil),并且清理两个map中的相关项。

4.当客户端断开连接时,对maps_from_conns_to_keys执行一下find,查看一下该连接是否是从被blpop/brpop阻塞的状态中退出的,如果是,则清理map。

6.在检查客户端keep_alive的部分(默认是60s超时就会断开),加了一个判断:如果在maps_from_conns_to_keys中能find到该客户端的对应记录,则不断开此连接,且更新其last_interaction属性

命令的行为:
image

TODO:

  • blpop这种阻塞式命令对binlog有影响,需要调整
    何时写binlog:在blpop,brpop中重写了doBinlog方法,只有真正发生写行为的时候才写一条binlog
    binlog写什么:对于blpop/brpop来说,当用户真正得到元素时,往binlog写一条对应的lpop/rpop,如果用户在得到元素前就主动断开,则不写binlog(因为pop行为实际上也没有发生)。
    产生的binlog:命令部分由小写字母组成(lpop/rpop), 而用户执行Lpop/Rpop命令产生的binlog中的命令都是大写(LPOP/RPOP),大小写不影响从节点消费,同时也可以作为一种区别方式(区分该条lpop/rpop binlog究竟是blpop/brpop产生的,还是用户执行Lpop/Rpop命令产生的)

  • 单测:已集成到Github Workflow中,每次pr都会触发。

    • 覆盖了redis单测中对blpop/brpop的用例,并且额外增加了不少用例, 测试时分别开了address/thread sanitizer,全部通过
      • 单个list不阻塞时的出列顺序(行为应当和lpop/rpop一样)
      • 解阻塞测试(超时自动解阻塞,lpush解阻塞,rpush解阻塞,rpoplpush解阻塞)
      • 并发下的阻塞/解阻塞测试
      • blpop/brpop多个list不阻塞时,从左到右选择第一个有元素的list进行pop
      • blpop/brpop的key中有重复key以及连续对同样的key进行blpop/brpop
      • 阻塞客户端的目标list被一条push增加了多个value,先完成多个value的入列再pop
      • 先被阻塞的先服务/阻塞最久的优先级最高
      • 主从复制测试(备注:rpoplpush命令本身主从同步有问题,所以测试里把对应部分先注释了)
      • 上述所有测试的多db并发运行
  • 代码整理:
    - 把方法体从.h移动到对应的.cpp
    - 删除测试用的打印输出
    - 使用clang-format整理格式(只整理了这个pr涉及到的文件)

给不应该被继承的cmd类都加上final关键字: 这个涉及到的类比较多,为了不对这个pr的可读性产生太大影响,决定下次提小pr的时候再一起做了(或者起个issue,也适合作为社区新人的first issue)

Fixes: #114

@cr-gpt
Copy link

cr-gpt bot commented May 23, 2023

Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information

include/pika_list.h Outdated Show resolved Hide resolved
include/pika_list.h Outdated Show resolved Hide resolved
src/net/src/dispatch_thread.h Outdated Show resolved Hide resolved
src/net/src/dispatch_thread.h Outdated Show resolved Hide resolved
src/net/src/dispatch_thread.h Outdated Show resolved Hide resolved
@wanghenshui
Copy link
Collaborator

还需要补充driver测试,用redis-py redisgo都行

@cheniujh
Copy link
Collaborator Author

还需要补充driver测试,用redis-py redisgo都行

好的,不过这几天有些事,可能要过个几天再加上

@AlexStocks
Copy link
Collaborator

@cheniujh please fix the file confliction.

include/pika_list.h Show resolved Hide resolved
wanghenshui
wanghenshui previously approved these changes Jul 6, 2023
include/pika_command.h Outdated Show resolved Hide resolved
include/pika_list.h Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
src/pika_command.cc Show resolved Hide resolved
@AlexStocks AlexStocks merged commit a1d45bf into OpenAtomFoundation:unstable Jul 19, 2023
8 checks passed
@cheniujh cheniujh deleted the new_add_blpop_brpop branch April 5, 2024 08:28
bigdaronlee163 pushed a commit to bigdaronlee163/pika that referenced this pull request Jun 8, 2024
* working on BlpopCmd::Doinitial

* working on : making the maps and new monitoring thread

* added bLRPop_blocking_info_ in pika_server.cc

* adding functions

* adding functions02

* temp save

* data structure done

* working

* revised data structure

* working on moving

* working

* next step is to add a timeout scan

* added TimerTaskManager

* modified TimerTaskManager(v2)

* scan added

* tiny fix

* changed the task of unblocking conn from sync to async

* removed some code for testing

* change from partition to db

* temp save for testing

* handle binlog of blr/pop(when conn get served, write a binlog of lpop/rpop)

* improved sanitizer options in CMakeLists

* temp save

* add unit test

* modified cmakelists

* temp save

* improved code based on reviwer's opinion

* renamed some variables

* 1. added record lock in ServeAndUnblockConns(void* args)
2. deleted testing code
3. formated code using clang-format

* added multi-db concurrency test

* renamed a flag from kCmdFlagsMayDfferWrite to kCmdFlagsMayDfferWriteBinlog

* 1. added record lock in ServeAndUnblockConns(void* args)
2. deleted testing code
3. formated code using clang-format
4. removed kCmdFlagsMayDeferWriteBinlog and override dobinlog in blpop and brpop

* removed unsed code

* revised code based on opinion of reviewer

* add the unit test to github workflow

* revised unit test file

* removed time counting

* clear github action cache

* revised based on reviwer's opinions

* Modify to adapt to Mac.

* Modify to adapt to Mac 2.

---------

Co-authored-by: cjh <1271435567@qq.com>
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.

redis API: support brpop, blpop
3 participants