-
Notifications
You must be signed in to change notification settings - Fork 0
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
test_gc_params integration test #6
base: feature/disk-object-storage-vfs
Are you sure you want to change the base?
test_gc_params integration test #6
Conversation
VFSGcRunsCompleted_start = node.query( | ||
"SELECT value FROM system.events WHERE event='VFSGcRunsCompleted'" | ||
) | ||
assert int(VFSGcRunsCompleted_start) <= 1 |
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.
Given GC start interval of 500ms, I'm confident this test will be marked as flaky in the CI. Maybe we could just record _start and _end and check the difference?
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.
The reason is, I believe, GC could run more than one time before server starts accepting connections
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.
We do check the difference, while the point is valid, I'll address 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.
@myrrc , I think that we can mitigate risk of 'spurious' GC invocations (caused by system.query_log whatever) moving 'default' from VFS, which is currently 'reacquire' for node 'node'.
I've done it in config_params.xml, but it should be done in config.xml as well.
Do you mind?
It seems it is the only way to make the test both reliable and meaningful.
We can mitigate risk of 'spurious' GC invocations (caused by
system.query_log whatever) moving 'default' from VFS, which is currently
'reacquire' for node 'node'.
A good solution, feel free to add a different storage policy for my tests
as well as change the default one.
|
test_gc_params integration test.
Covers vfs_batch_can_wait_ms, vfs_batch_min_size and event VFSGcRunsCompleted