allthingssecurity commented on PR #28136: URL: https://github.com/apache/camel/pull/28136#issuecomment-6285513385
Thanks for the review. I went with option (a), in 00f39093950e: - The first time a producer uploads a window, it lists the window's objects (`ListObjectsV2` with the window key as prefix, following continuation tokens) and starts the count at the highest upload found: the first upload's name counts 1, `-N` counts N+1. Other keys under that prefix (another extension, `-x`) are ignored. After that the count is kept in memory as before. That's one listing per window per producer. - If the listing fails (for example without `s3:ListBucket`), the producer logs a WARN and numbers the window from the first upload, as before this PR. The streaming docs and the upgrade guide say so. - `doStop` now counts the windows it flushes, so a stopped and restarted producer instance does not reuse the name either. - New tests in `AWS2S3StreamUploadTimestampWindowOverwriteTest`: a restart in the middle of a window (stop flushes `..._0800-0805.txt`, a new producer writes `..._0800-0805-1.txt`), objects `-1`, `-10`, `-2` already in the bucket (next is `-11`, with the listing paged two keys at a time), and a listing that fails with 403. With the listing removed, the two restart tests fail. The camel-aws2-s3 unit tests pass: 110 run, 0 failures. - Docs: producers running at the same time with the same `keyName` still need distinct key names. Option (a) does not prevent that collision. Backport: agreed, main only. CI: `S3StreamUploadTimestampGroupingIT.testTimestampGroupingBasic` and `S3StreamUploadTimestampGroupingEdgeCasesIT.testTimestampGroupingWithVeryLargeWindow` send 10 messages per window with `batchMessageNumber=5`. They expected one object per window, which was the overwrite: on main, half of the messages are lost. They now check the exact keys (`basicTest_20240101_0800_0800-0805.txt` and `...-0805-1.txt`, the same for 0805-0810, and `largeWindowTest_20240101_0800_0800-0900.txt` and `-1.txt`). I could not run the ITs locally (no Docker here), so CI will be their first run. _Claude Code on behalf of allthingssecurity_ -- 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]
