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(tests): Handle early return from influxdb create database #5928

Merged
merged 7 commits into from Jan 8, 2021

Conversation

bruceg
Copy link
Member

@bruceg bruceg commented Jan 7, 2021

Sometimes, the influxdb v1 server will return early from creating the
database, providing an OK response code before the database is actually
created. Subsequent sink calls will then encounter a NOT_FOUND response
when trying to write to the database, resulting in a spurious failure.
This tests for database existence after creating it by doing an empty
write, and looping until that returns NO_CONTENT.

Closes #5612

Signed-off-by: Bruce Guenter bruce@timber.io

Bruce Guenter added 5 commits January 7, 2021 16:07
Signed-off-by: Bruce Guenter <bruce@timber.io>
Signed-off-by: Bruce Guenter <bruce@timber.io>
Signed-off-by: Bruce Guenter <bruce@timber.io>
Signed-off-by: Bruce Guenter <bruce@timber.io>
Some times, the influxdb v1 server will return early from creating the
database, providing an OK response code before the database is actually
created. Subsequent sink calls will then encounter a NOT_FOUND response
when trying to write to the database, resulting in a spurious failure.
This tests for database existence after creating it by doing an empty
write, and looping until that returns NO_CONTENT.

Signed-off-by: Bruce Guenter <bruce@timber.io>
@bruceg bruceg added type: bug A code related bug. domain: tests Anything related to Vector's internal tests sink: influxdb_metrics Anything `influxdb_metrics` sink related sink: prometheus_remote_write Anything `prometheus_remote_write` sink related labels Jan 7, 2021
@bruceg bruceg requested a review from jszwedko January 7, 2021 22:54
@bruceg bruceg self-assigned this Jan 7, 2021
Signed-off-by: Bruce Guenter <bruce@timber.io>
Copy link
Member

@jszwedko jszwedko left a comment

Choose a reason for hiding this comment

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

Nice! Looks good. I like the approach of looping until it is available.

I do think it'd be good to cap the loop so tests don't hang forever in extreme circumstances. You might just be able to use test_util::wait_for_duration? https://github.com/timberio/vector/blob/411100307f44d1d73933b0080dca1da254cd914d/src/test_util/mod.rs#L297-L309

// accept writes to the database, leading to test failures. Test
// this with empty writes and loop if it reports the database
// does not exist yet.
loop {
Copy link
Member

Choose a reason for hiding this comment

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

I think it'd be good to have this loop break after, say, 10 iterations (5 seconds) to avoid this test hanging forever.

Signed-off-by: Bruce Guenter <bruce@timber.io>
@bruceg bruceg merged commit 08bb58f into master Jan 8, 2021
@bruceg bruceg deleted the fix-prometheus-remote-write-test-failure branch January 8, 2021 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain: tests Anything related to Vector's internal tests sink: influxdb_metrics Anything `influxdb_metrics` sink related sink: prometheus_remote_write Anything `prometheus_remote_write` sink related type: bug A code related bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sinks::prometheus::remote_write::integration_tests::insert_metrics_over_http CI failure
2 participants