You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "pjfanning (via GitHub)" <gi...@apache.org> on 2026/10/25 00:22:59 UTC
[PR] Don't log every DB call when log-db-calls-exceeding is disabled [pekko-persistence-r2dbc]
pjfanning opened a new pull request, #522:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/522
### Motivation
When `logDbCallsExceeding` is negative (`log-db-calls-exceeding = off`, or the `-1.millis` passed by `R2dbcSession.withSession`), `durationInMicros` returned `0`, so every `durationMicros >= logDbCallsExceedingMicros` check was true and getConnection, select and update calls were all logged at INFO.
### Modification
Return `Long.MinValue` from `durationInMicros` when timing is disabled, matching akka/akka-persistence-r2dbc#328 (Apache 2.0 licensed since its change date).
### Result
No per-call INFO logging when `log-db-calls-exceeding` is disabled; logging with a non-negative threshold is unchanged.
### Tests
- `sbt "core/testOnly org.apache.pekko.persistence.r2dbc.internal.R2dbcExecutorSpec"` against PostgreSQL: passed. The new disabled-logging test fails without the fix.
- `sbt core/mimaReportBinaryIssues`: passed
### References
Fixes #521
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] Don't log every DB call when log-db-calls-exceeding is disabled [pekko-persistence-r2dbc]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #522:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/522#issuecomment-6415135412
Merged but I'll follow up if anyone has further requests
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] Don't log every DB call when log-db-calls-exceeding is disabled [pekko-persistence-r2dbc]
Posted by "ptrdom (via GitHub)" <gi...@apache.org>.
ptrdom commented on code in PR #522:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/522#discussion_r4375654791
##########
core/src/main/scala/org/apache/pekko/persistence/r2dbc/internal/R2dbcExecutor.scala:
##########
@@ -134,7 +134,9 @@ class R2dbcExecutor(
private def nanoTime(): Long =
if (logDbCallsExceedingEnabled) System.nanoTime() else 0L
- private def durationInMicros(startTime: Long): Long = (nanoTime() - startTime) / 1000
+ // Long.MinValue when disabled, so that `durationMicros >= logDbCallsExceedingMicros` is never true
+ private def durationInMicros(startTime: Long): Long =
+ if (logDbCallsExceedingEnabled) (nanoTime() - startTime) / 1000 else Long.MinValue
Review Comment:
Nit (optional): in the enabled branch `nanoTime()` re-checks `logDbCallsExceedingEnabled`; `System.nanoTime() - startTime` would be a bit more direct.
##########
core/src/test/scala/org/apache/pekko/persistence/r2dbc/internal/R2dbcExecutorSpec.scala:
##########
@@ -82,6 +84,43 @@ class R2dbcExecutorSpec
"R2dbcExecutor" should {
+ "not log db calls when log-db-calls-exceeding is disabled" in {
+ val executor = new R2dbcExecutor(
+ connectionFactoryProvider,
+ LoggerFactory.getLogger(getClass),
+ logDbCallsExceeding = -1.millis,
+ closeCallsExceeding)(typedSystem.executionContext, typedSystem)
+
+ LoggingTestKit.info("getConnection took").withOccurrences(0).expect {
+ LoggingTestKit.info("Selected").withOccurrences(0).expect {
+ LoggingTestKit.info("DB call completed").withOccurrences(0).expect {
+ executor
+ .select("test")(
+ _.createStatement(s"select col from $table"),
+ row => Row(row.get("col", classOf[String])))
+ .futureValue
+ executor.updateOne("test")(_.createStatement(s"delete from $table where col = 'x'")).futureValue
+ }
+ }
+ }
+ }
+
+ "log db calls when log-db-calls-exceeding is zero" in {
+ val executor = new R2dbcExecutor(
+ connectionFactoryProvider,
+ LoggerFactory.getLogger(getClass),
+ logDbCallsExceeding = Duration.Zero,
+ closeCallsExceeding)(typedSystem.executionContext, typedSystem)
+
+ LoggingTestKit.info("Selected").expect {
Review Comment:
Nit (optional): this only asserts the select log. Also expecting `getConnection took` and `DB call completed` (via an `updateOne`) would mirror the disabled test and cover all log sites on the enabled path.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] Don't log every DB call when log-db-calls-exceeding is disabled [pekko-persistence-r2dbc]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #522:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/522#discussion_r4375806574
##########
core/src/main/scala/org/apache/pekko/persistence/r2dbc/internal/R2dbcExecutor.scala:
##########
@@ -134,7 +134,9 @@ class R2dbcExecutor(
private def nanoTime(): Long =
if (logDbCallsExceedingEnabled) System.nanoTime() else 0L
- private def durationInMicros(startTime: Long): Long = (nanoTime() - startTime) / 1000
+ // Long.MinValue when disabled, so that `durationMicros >= logDbCallsExceedingMicros` is never true
+ private def durationInMicros(startTime: Long): Long =
+ if (logDbCallsExceedingEnabled) (nanoTime() - startTime) / 1000 else Long.MinValue
Review Comment:
Done in a3cb6dc: the enabled branch now uses `System.nanoTime() - startTime` directly.
##########
core/src/test/scala/org/apache/pekko/persistence/r2dbc/internal/R2dbcExecutorSpec.scala:
##########
@@ -82,6 +84,43 @@ class R2dbcExecutorSpec
"R2dbcExecutor" should {
+ "not log db calls when log-db-calls-exceeding is disabled" in {
+ val executor = new R2dbcExecutor(
+ connectionFactoryProvider,
+ LoggerFactory.getLogger(getClass),
+ logDbCallsExceeding = -1.millis,
+ closeCallsExceeding)(typedSystem.executionContext, typedSystem)
+
+ LoggingTestKit.info("getConnection took").withOccurrences(0).expect {
+ LoggingTestKit.info("Selected").withOccurrences(0).expect {
+ LoggingTestKit.info("DB call completed").withOccurrences(0).expect {
+ executor
+ .select("test")(
+ _.createStatement(s"select col from $table"),
+ row => Row(row.get("col", classOf[String])))
+ .futureValue
+ executor.updateOne("test")(_.createStatement(s"delete from $table where col = 'x'")).futureValue
+ }
+ }
+ }
+ }
+
+ "log db calls when log-db-calls-exceeding is zero" in {
+ val executor = new R2dbcExecutor(
+ connectionFactoryProvider,
+ LoggerFactory.getLogger(getClass),
+ logDbCallsExceeding = Duration.Zero,
+ closeCallsExceeding)(typedSystem.executionContext, typedSystem)
+
+ LoggingTestKit.info("Selected").expect {
Review Comment:
Done in a3cb6dc: the zero-threshold test now also expects `getConnection took` (twice, once for the select and once for an `updateOne`) and `DB call completed`, matching the disabled test.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] Don't log every DB call when log-db-calls-exceeding is disabled [pekko-persistence-r2dbc]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning merged PR #522:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/522
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org