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 repeat reset bucket #2945

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mkit777
Copy link

@mkit777 mkit777 commented Nov 8, 2022

Describe what this PR does / why we need it

现有实现方式存在重复重置 bucket 的可能

  1. 假设有 a, b 两个线程,同时满足 windowStart > old.windowStart() 条件
  2. a 线程获取锁成功,并完成重置,释放锁
  3. 此时b线程开始尝试获取锁,由于a线程已释放,所以可以获取成功,重置bucket
    综上,会发生重复重置 bucket 的可能

Does this pull request fix one issue?

None

Describe how you did it

double check,当发现不满足时通过 continue 跳过重置逻辑,在下一轮即可获取正确的 bucket

@CLAassistant
Copy link

CLAassistant commented Nov 8, 2022

CLA assistant check
All committers have signed the CLA.

@sczyh30 sczyh30 added the area/metrics Issues or PRs related to metrics and monitoring label Nov 9, 2022
@sczyh30
Copy link
Member

sczyh30 commented Nov 9, 2022

tryLock 为非等待性锁,a 线程获取成功时,b 线程会进入自旋 重新尝试;当 reset 完成后后续线程不会进入到这个分支中。

@mkit777
Copy link
Author

mkit777 commented Nov 9, 2022

tryLock 是非等待锁,但是否存在下面这种可能:T1、T2 同时满足了 if 条件,准备执行 tryLock,T1顺利的执行完了 tryLock、 resetWindowTo、unlock,而 T2 线程由于某些原因,导致获取锁的操作比较靠后,直到 T1 unlock 后 才开始 tryLock,此时锁是可以获取成功的,这就造成了重复重置 bucket。具体流程如下图:

image

@yanxianchao
Copy link

我认为,这是一个问题,获得锁的线程要再判断一下,原来的条件(windowStart > old.windowStart())是否还满足,如果不满足,就要再自旋一次。简单来说:只有拿到锁以后的满足条件,才是真正的满足条件。

@yanxianchao
Copy link

这个类(LeapArray)有一些线程安全问题,和issues2949是类似的问题

@sczyh30 sczyh30 added the to-review To review label Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics Issues or PRs related to metrics and monitoring to-review To review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants