david-streamlio opened a new pull request, #86:
URL: https://github.com/apache/pulsar-connectors/pull/86
Fixes #85
### Motivation
An audit for timing flakes found the same defect in four more modules. Each
writes to a sink that flushes **asynchronously**, waits a **fixed**
`Thread.sleep`, then asserts the flush happened. On a slow runner the flush has
not run and the assertion fails — the sink is fine, the test raced it.
This shape has already failed two unrelated PRs today:
- `InfluxDBSinkTest.testOpenWriteCloseJson` (v2) broke #39 → fixed in #73
- `PulsarSchemaHistoryTest.setup` broke #79 → fixed in #84
### Findings
| Module | Test | Sleep | Then asserts | Async mechanism |
|---|---|---|---|---|
| `hdfs3` | `HdfsSequentialSinkTest.write5000Test` | 2000 ms | `verify(...,
times(5000)).ack()` | `syncInterval` background sync |
| `hdfs3` | `HdfsSequentialSinkTest`, `HdfsTextSinkTest` | 2000 ms |
`verify(..., times(100)).ack()` | same |
| `mongo` | `MongoSinkTest` × 5 sites | 1000 ms | `verify(...,
times(n)).ack()/.fail()` | `flushExecutor.scheduleAtFixedRate` |
| `influxdb` v1 | `InfluxDBGenericRecordSinkTest` | 1000 ms |
`verify(influxDB, times(1)).write(...)` | `BatchSink` flush executor — the very
class whose v2 sibling flaked |
| `hbase` | `HbaseGenericRecordSinkTest` | 500 ms | reads the row back |
`HbaseAbstractSink` batch flush |
`write5000Test` is the sharpest: 5000 acks asserted within a hard-coded 2
seconds.
### Modifications
Replace each fixed sleep with an Awaitility poll on the same condition.
**The assertions are unchanged** — only the waiting is. `awaitility` is already
on every module's test classpath via the shared java-conventions plugin. The
tests also get faster, since a poll returns as soon as the condition holds
rather than always sleeping the full duration.
### Verifying this change
All four suites pass: `./gradlew :mongo:test :influxdb:test :hdfs3:test
:hbase:test` → `BUILD SUCCESSFUL`.
More importantly, the race is **demonstrated**, not asserted. Taking
master's `MongoSinkTest` and shortening its sleep from 1000 ms to 1 ms —
simulating a runner slow enough that the flush has not fired — reproduces the
CI signature exactly:
```
MongoSinkTest > testWriteBadMessage FAILED
Wanted but not invoked: ...
MongoSinkTest > testWriteGoodMessage FAILED
Wanted but not invoked: ...
```
The Awaitility version passes with **no sleep at all**, because it waits on
the condition rather than on a guessed duration.
### Notes
- `HbaseGenericRecordSinkTest` is `@Test(enabled = false)` and does not run
today, so its change is compile-only. Included so the pattern is not
reintroduced when it is re-enabled.
- Deliberately **not** changed, with reasons, in #85: `redis` (sleeps but
asserts nothing afterwards), `jdbc/sqlite` (sleep inside a retry loop),
`azure-data-explorer` (credential-gated, never runs in CI — see #49), and
`file` (its sleeps are deliberate soak windows; its races were fixed in #41 and
#71).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
