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.

Sign up for GitHub

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

Jump to bottom

skip the wait in rate limiter in CI #2795

Merged
merged 3 commits into from
Sep 7, 2021
Merged

skip the wait in rate limiter in CI #2795

merged 3 commits into from
Sep 7, 2021

Conversation

critical27
Copy link
Contributor

@critical27 critical27 commented Sep 6, 2021 •
edited
Loading

Skip the first second wait in rate limiter, which will take 1 second in every case in TCK tests. And then, another core occur, see #2794, remove the DCHECK temporally.

Duration of TCK should back to normal.

TCK with ASAN still time out, probably because of spin lock. Still working on it.

image

@critical27 critical27 added the ready-for-testing PR: ready for the CI test label Sep 6, 2021
@critical27 critical27 mentioned this pull request Sep 6, 2021
Closed
@yixinglu
Copy link
Contributor

yixinglu commented Sep 6, 2021

Good catch. But it's still spend too much time in CI. Maybe what else is still hurting our CI

bright-starry-sky
bright-starry-sky previously approved these changes Sep 6, 2021
View reviewed changes
Copy link
Contributor

@bright-starry-sky bright-starry-sky left a comment

Choose a reason for hiding this comment

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

LGTM .

@critical27
Copy link
Contributor Author

Good catch. But it's still spend too much time in CI. Maybe what else is still hurting our CI

Yeah, CentOS takes about 6 minutes to run task, fix them one by one.

yixinglu
yixinglu previously approved these changes Sep 7, 2021
View reviewed changes
@critical27
Copy link
Contributor Author

Hold on for on sec, let me fix the DCHECK

@critical27
Copy link
Contributor Author

critical27 commented Sep 7, 2021 •
edited
Loading

Close #2794, #2799, #2783.

#2790 may or may not be related, need to check later.

Ready to merge

This was linked to issues Sep 7, 2021
Core in rebuild index #2794
Closed
Storage crash when running tck #2783
Closed
RateLimiter crash #2799
Closed
@critical27 critical27 requested a review from liuyu85cn September 7, 2021 09:08
@bright-starry-sky
Copy link
Contributor

LGTM

@bright-starry-sky bright-starry-sky merged commit d742768 into vesoft-inc:master Sep 7, 2021
@critical27 critical27 deleted the fix branch September 8, 2021 04:18
@jamieliu1023 jamieliu1023 mentioned this pull request Sep 11, 2021
Closed
This was referenced Sep 17, 2021
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bright-starry-sky bright-starry-sky bright-starry-sky approved these changes

@CPWstatic CPWstatic CPWstatic approved these changes

@yixinglu yixinglu yixinglu left review comments

@liuyu85cn liuyu85cn Awaiting requested review from liuyu85cn

Assignees
No one assigned
Labels
ready-for-testing PR: ready for the CI test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

RateLimiter crash Core in rebuild index Storage crash when running tck
4 participants
@critical27 @yixinglu @bright-starry-sky @CPWstatic