Skip to content

[improve][meta] Fix busy-loop causes watcher can't acquire lock.#19769

Merged
mattisonchao merged 11 commits intoapache:masterfrom
mattisonchao:fix/test/zksession
Mar 13, 2023
Merged

[improve][meta] Fix busy-loop causes watcher can't acquire lock.#19769
mattisonchao merged 11 commits intoapache:masterfrom
mattisonchao:fix/test/zksession

Conversation

@mattisonchao
Copy link
Member

@mattisonchao mattisonchao commented Mar 9, 2023

Fixes #11032

Master Issue: #11032

Motivation

Due to the session timeout time being too small, it will cause the scheduler to be in busy loop status and give no chance for other threads(process method) to get the synchronous lock.
This behaviour may affect the notification order and make the test fail.

Modifications

  • Use scheduleWithFixedDelay to avoid a busy loop.
  • Make the test get out of quarantine.

Verifying this change

  • Make sure that the change passes the CI checks.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@mattisonchao mattisonchao self-assigned this Mar 9, 2023
@mattisonchao mattisonchao requested a review from lhotari March 9, 2023 15:39
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Mar 9, 2023
@mattisonchao mattisonchao added this to the 3.0.0 milestone Mar 9, 2023
@mattisonchao mattisonchao reopened this Mar 9, 2023
@lhotari
Copy link
Member

lhotari commented Mar 9, 2023

@mattisonchao isn't this a problem that also impacts our maintenance branches and actual production code?

@mattisonchao
Copy link
Member Author

@lhotari
In production, the default value is 30sec. I think it just affects the test. But it's not stringent since users can set whatever they like.

Copy link
Member

@coderzc coderzc left a comment

Choose a reason for hiding this comment

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

LGTM

Also fix the same issue in the EtcdSessionWatcher?

@mattisonchao
Copy link
Member Author

LGTM

Also fix the same issue in the EtcdSessionWatcher?

I don't think so, because ETCD has no notification mechanism. Therefore, I think ETCD Session watcher has no problem.

@mattisonchao mattisonchao changed the title [fix][meta] Fix busy-loop causes watcher can't acquire lock. [improve][meta] Fix busy-loop causes watcher can't acquire lock. Mar 12, 2023
@mattisonchao mattisonchao merged commit 5caca1a into apache:master Mar 13, 2023
@mattisonchao mattisonchao deleted the fix/test/zksession branch March 13, 2023 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky-test: ZKSessionTest.testReacquireLeadershipAfterSessionLost

5 participants