minjibir commented on PR #489: URL: https://github.com/apache/pekko-persistence-r2dbc/pull/489#issuecomment-5820039332
**1. One `db_timestamp` per flush can break `eventsBySlices`** Fixed. Each write request now gets its own strictly increasing microsecond timestamp at flush time, from an `AtomicLong` cursor over `InstantFactory.now()`. No two requests share a timestamp, which keeps `BySliceQuery`'s seen-map bounded to 1 inside the journal actor. `publish()` passes each request's stored timestamp, so pub-sub offsets match the database rows. See [R2dbcBatchJournalPublishTimestampSpec](core/src/test/scala/org/apache/pekko/persistence/r2dbc/journal/R2dbcBatchJournalPublishTimestampSpec.scala#L87-L137). **2. Timestamp is taken before the connection is acquired** Documented. `behind-current-time` lives under the query config, so it can't be set from the journal plugin block. I added a comment in [reference.conf](core/src/main/resources/reference.conf#L76-L80) under `batched-journal` and an `application.conf` note in [journal.md](docs/src/main/paradox/journal.md#L77-L79): raise `pekko.persistence.r2dbc.query.behind-current-time` well above expected connection-acquisition latency when the pool is contended. **3. A stale `FlushDone` can survive an actor restart** Fixed. `FlushDone` now carries an incarnation generation (`FlushDone(generation: Long)`) and the handler drops messages from older incarnations. Test: [R2dbcBatchJournalBatchingSpec](core/src/test/scala/org/apache/pekko/persistence/r2dbc/journal/R2dbcBatchJournalBatchingSpec.scala#L182-L232), class `R2dbcBatchJournalStaleFlushDoneSpec`. **4. Mixed-pid precondition is documented but not enforced** Enforcing this in `JournalDao.writeEvents` would change shared core internals and existing `R2dbcJournal` behavior. I'd rather stay inside the scope agreed in #471 and avoid drift from the Akka codebase, especially since `R2dbcBatchJournal` already enforces `db-timestamp-monotonic-increasing = on` at construction. Say the word if you still want the guard in `JournalDao` and I'll add it. **5. `publish` failures are silently swallowed** Fixed. Each request in `publish()` is wrapped in `try / catch NonFatal` with a warning log that includes the persistence ID. One failed publish no longer skips the rest of the batch or escapes into `recoverWith`. **6. `max-batch-time` is unvalidated** Fixed. Added `require(maxBatchTime > Duration.Zero)` in `R2dbcBatchJournal`, documented the bound in `reference.conf` and `journal.md`, and added zero/negative-duration fail-fast tests in [R2dbcBatchJournalValidationSpec](core/src/test/scala/org/apache/pekko/persistence/r2dbc/journal/R2dbcBatchJournalValidationSpec.scala#L191-L221). -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
