-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
flaky onlineddl tests: reduce -online_ddl_check_interval #7847
flaky onlineddl tests: reduce -online_ddl_check_interval #7847
Conversation
Signed-off-by: Shlomi Noach <[email protected]>
Hmmm actually the problem is more specific to |
Signed-off-by: Shlomi Noach <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
ugh, now failing: https://github.com/vitessio/vitess/pull/7847/checks?check_run_id=2331837611 . I'll keep looking into. |
The nature of this error is unclear: |
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Now, it fails on a nonsensical issue: the very first This does not reproduce locally, ever. All Also, once in a while, another
This feels like resource exhaustion. I have no other explanation to this behavior. |
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
bc1ffd6
to
d7f0e8d
Compare
rebased and removed the os tune up commit since we have a different fix for CI now |
@shlomi-noach: tests are passing. I see you moved it to Draft: is it ready to be merged? |
I moved it to |
Signed-off-by: Shlomi Noach <[email protected]>
…s into flaky-tests-online-ddl Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Some update on where In
I invested time to verify/deby that the problem is with Reiterating that all these tests pass just fine on multiple environemtns, just no in GitHub CI. BTW, the test passes in #7850. So I'm still looking for a platform problem. Sockets/files/other resources. |
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
The
Running even more experiments to double validate this. |
Signed-off-by: Shlomi Noach <[email protected]>
If I remove hooks, then the next thing It's all about ports. |
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Was it intentional to checkin a new gh-ost binary?
- When we merge this, we should squash-merge since so many of the commits were experimental.
@@ -12,6 +12,16 @@ jobs: | |||
with: | |||
go-version: 1.15 | |||
|
|||
- name: Tune the OS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need the port range fix? Or is the /etc/hosts
fix sufficient?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This had immediate positive effect last week on local_example
tests. I'll experiment without it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed, and tests are green!
No! It was experimental. Reverting.
Cool |
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Removing the port range patch, 8f4444c, seems to look good. Tests are still passing without the patch. |
Suqash commit: 2179d0c |
Description
Solves #7834
Not sure how this happened, will later look in
git blame
and history, but in the tests suite,vtctld
should run with a very short-online_ddl_check_interval
. For some reason the flag didn't appear in any of the online ddl tests. It must have changes somehow over time, looking into. anyway, this should resolve the current CI failures seen mostly inonlineddl_ghost
but also in otheronlineddl_*
tests.Related Issue(s)
Checklist
Deployment Notes
Impacted Areas in Vitess
Components that this PR will affect: