minjibir commented on PR #489: URL: https://github.com/apache/pekko-persistence-r2dbc/pull/489#issuecomment-5610505293
All review points are addressed in 8a91cdf and 446fbcb; the ones fixed exactly as suggested are resolved in their threads. Below are the points where I took a different route. **B2 (max-batch-size bounds requests, not rows) and B3 (bisection cost):** I looked at these against the default journal and neither is introduced here. `R2dbcJournal.asyncWriteMessages` has the same code path where one request can hold an unbounded number of events, a large `persistAll` or a `persistAsync` burst grouped into one AtomicWrite, and `JournalDao.writeEvents` has no row or byte cap in either journal. A row or byte cap would have to live in `JournalDao` and change default-journal behavior, which is out of scope and breaks the additive constraint from #471. Batching amplifies the probability of a very large statement because a flush sums up to `max-batch-size` requests, and bisection costs at most about 2n statements for n = max-batch-size, which is the count the default journal would have used for the same writes anyway. The fix here is documentation: Memory now states a request can hold an arbitrary event count and neither journal caps it; Failures now states the worst-case retry cost; and the plugin is explicitly scoped to many small writes, matching the measurements in #471 where the single insert wins above roughly 64 KB. **B4 (raw config accessors):** Deliberately kept as `config.getInt`/`getDuration`. Routing through `JournalSettings` would modify an existing shared class for three plugin-local keys. The design constraint from #471, and the basis on which the prototype was accepted, is that this feature adds one file and touches nothing else, so akka-persistence-r2dbc upstream syncs stay conflict-free. **C, duplication with R2dbcJournal (~150 lines):** Kept on purpose. The copy is the design from #471: a self-contained class that reuses existing internals without changing them. Extracting a shared base trait would edit `R2dbcJournal`, which is exactly the maintenance cost the issue discussion asked to avoid. One heads-up on CI: with the dialect requirement from A4 in place, the batched journal specs need a MySQL skip in the matrix; a follow-up commit will gate those suites on the postgres or yugabyte dialect so the MySQL jobs run clean. -- 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]
