oxidecomputer / oxidecomputer/omicron
Another possible race in `oximeter` integration tests
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 572
- Forks
- 97
- Avg merge
- 2d 12h
- Merged PRs (30d)
- 96
Description
@davepacheco saw this test failure. The relevant bit is here:
4703 | 2022-05-03T18:56:15.474Z | failures:
-- | -- | --
4704 | 2022-05-03T18:56:15.480Z |
4705 | 2022-05-03T18:56:15.487Z | ---- integration_tests::oximeter::test_oximeter_reregistration stdout ----
4706 | 2022-05-03T18:56:15.493Z | log file: "/var/tmp/omicron_tmp/test_all-1557dfd33414e1a3-test_oximeter_reregistration.5150.46.log"
4707 | 2022-05-03T18:56:15.499Z | note: configured to log to "/var/tmp/omicron_tmp/test_all-1557dfd33414e1a3-test_oximeter_reregistration.5150.46.log"
4708 | 2022-05-03T18:56:15.504Z | thread 'integration_tests::oximeter::test_oximeter_reregistration' panicked at 'assertion failed: `(left == right)`
4709 | 2022-05-03T18:56:15.510Z | left: `Target { name: "integration_target", fields: [Field { name: "name", value: String("integration-test-target") }, Field { name: "name", value: String("integration-test-target") }] }`,
4710 | 2022-05-03T18:56:15.516Z | right: `Target { name: "integration_target", fields: [Field { name: "name", value: String("integration-test-metric") }, Field { name: "name", value: String("integration-test-metric") }] }`', nexus/tests/integration_tests/oximeter.rs:308:5
4711 | 2022-05-03T18:56:15.521Z | note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
4712 | 2022-05-03T18:56:15.527Z | WARN: dropped CockroachInstance without cleaning it up first (there may still be a child process running and a temporary directory leaked)
4713 | 2022-05-03T18:56:15.534Z | WARN: temporary directory leaked: /var/tmp/omicron_tmp/.tmpKT5h8q
4714 | 2022-05-03T18:56:15.539Z | WARN: dropped ClickHouseInstance without cleaning it up first (there may still be a child process running and a temporary directory leaked)
4715 | 2022-05-03T18:56:15.545Z |
4716 | 2022-05-03T18:56:15.551Z |
4717 | 2022-05-03T18:56:15.557Z | failures:
4718 | 2022-05-03T18:56:15.565Z | integration_tests::oximeter::test_oximeter_reregistration
That assertion should never be hit. We store the field names for a timeseries target and metric in the order they're declared in the Rust struct that defines them. Those are stored in a ClickHouse table called oximeter.timeseries_schema, which includes the fields as nested table. When selecting a row with a nested table, we get back an array for each of the nested columns, guaranteed to be the same length.
Looking at the exact error message, this is quite confusing. This is expected to show that the names of the fields are the same in both cases, since they're not changing during the test. That is we'd, expect two arrays ["foo", "bar"] in both cases. When I first read this, I thought it showed the arrays reversed in the two cases, e.g., ["foo", "bar"] in one and ["bar", "foo"] in the other. What it actually shows is ["foo", "foo"] and ["bar", "bar"]. I'm really not sure how this can even happen, as there is no row in the database with a nested field that has either of those names together. This appears to be consistent with the idea that query returns rows while they're being sorted. (I'm not saying that's what's going on.)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with nexus/tests/integration_tests/oximeter.rs at the failing assertion around line 308 and run integration_tests::oximeter::test_oximeter_reregistration to reproduce the failure. Read oximeter/db/src/db-init.sql around line 152 and investigate how the nested timeseries_schema fields are queried. Done means the suspected race is understood and the test no longer produces inconsistent field arrays.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- clickhouse, rust
- Domain
- databases, testing
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100