You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "ptrdom (via GitHub)" <gi...@apache.org> on 2024/11/18 16:07:02 UTC
[PR] Projection implementation for MySQL support [pekko-persistence-r2dbc]
ptrdom opened a new pull request, #177:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/177
Related to https://github.com/apache/pekko-persistence-r2dbc/issues/158.
--
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] Projection implementation for MySQL support [pekko-persistence-r2dbc]
Posted by "ptrdom (via GitHub)" <gi...@apache.org>.
ptrdom commented on code in PR #177:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/177#discussion_r1846881840
##########
projection/src/main/scala/org/apache/pekko/projection/r2dbc/R2dbcProjectionSettings.scala:
##########
Review Comment:
Of course this class is different from `R2dbcSettings` in a way that users can manually construct it - it is not constructed by config only, so will have to keep the means to set/modify its attributes.
--
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] Projection implementation for MySQL support [pekko-persistence-r2dbc]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #177:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/177#issuecomment-2508924528
Thanks. I merged this. We can do some tweaks in new PRs if anyone has any issues with these changes.
--
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] Projection implementation for MySQL support [pekko-persistence-r2dbc]
Posted by "ptrdom (via GitHub)" <gi...@apache.org>.
ptrdom commented on code in PR #177:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/177#discussion_r1848839523
##########
projection/src/main/scala/org/apache/pekko/projection/r2dbc/R2dbcProjectionSettings.scala:
##########
Review Comment:
@pjfanning Do you have a preferred solution for this?
--
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] Projection implementation for MySQL support [pekko-persistence-r2dbc]
Posted by "ptrdom (via GitHub)" <gi...@apache.org>.
ptrdom commented on code in PR #177:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/177#discussion_r1856836038
##########
projection/src/main/scala/org/apache/pekko/projection/r2dbc/internal/mysql/MySQLR2dbcOffsetStore.scala:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pekko.projection.r2dbc.internal.mysql
+
+import java.time.Clock
+
+import org.apache.pekko
+import pekko.actor.typed.ActorSystem
+import pekko.annotation.InternalApi
+import pekko.persistence.r2dbc.internal.R2dbcExecutor
+import pekko.persistence.r2dbc.internal.Sql.DialectInterpolation
+import pekko.projection.BySlicesSourceProvider
+import pekko.projection.ProjectionId
+import pekko.projection.r2dbc.R2dbcProjectionSettings
+import pekko.projection.r2dbc.internal.R2dbcOffsetStore
+
+/**
+ * INTERNAL API
+ */
+@InternalApi
+private[projection] class MySQLR2dbcOffsetStore(
+ projectionId: ProjectionId,
+ sourceProvider: Option[BySlicesSourceProvider],
+ system: ActorSystem[_],
+ settings: R2dbcProjectionSettings,
+ r2dbcExecutor: R2dbcExecutor,
+ clock: Clock = Clock.systemUTC())
+ extends R2dbcOffsetStore(projectionId, sourceProvider, system, settings, r2dbcExecutor, clock) {
+
+ override lazy val timestampSql: String = "NOW(6)"
Review Comment:
We already have an ADT for `Dialect`, I guess more FP-ish way would be to add a private extension class with these methods, although these kinds of attributes are already present in `core`.
--
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] Projection implementation for MySQL support [pekko-persistence-r2dbc]
Posted by "ptrdom (via GitHub)" <gi...@apache.org>.
ptrdom commented on code in PR #177:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/177#discussion_r1855160442
##########
projection/src/main/scala/org/apache/pekko/projection/r2dbc/R2dbcProjectionSettings.scala:
##########
@@ -44,25 +46,149 @@ object R2dbcProjectionSettings {
keepNumberOfEntries = config.getInt("offset-store.keep-number-of-entries"),
evictInterval = config.getDuration("offset-store.evict-interval"),
deleteInterval = config.getDuration("offset-store.delete-interval"),
- logDbCallsExceeding)
+ logDbCallsExceeding
+ )
}
def apply(system: ActorSystem[_]): R2dbcProjectionSettings =
apply(system.settings.config.getConfig(DefaultConfigPath))
+
+ def apply(
+ schema: Option[String],
+ offsetTable: String,
+ timestampOffsetTable: String,
+ managementTable: String,
+ useConnectionFactory: String,
+ timeWindow: JDuration,
+ keepNumberOfEntries: Int,
+ evictInterval: JDuration,
+ deleteInterval: JDuration,
+ logDbCallsExceeding: FiniteDuration
+ ): R2dbcProjectionSettings = new R2dbcProjectionSettings(
+ Dialect.Postgres,
+ schema,
+ offsetTable,
+ timestampOffsetTable,
+ managementTable,
+ useConnectionFactory,
+ timeWindow,
+ keepNumberOfEntries,
+ evictInterval,
+ deleteInterval,
+ logDbCallsExceeding
+ )
}
-// FIXME remove case class, and add `with` methods
-final case class R2dbcProjectionSettings(
- schema: Option[String],
- offsetTable: String,
- timestampOffsetTable: String,
- managementTable: String,
- useConnectionFactory: String,
- timeWindow: JDuration,
- keepNumberOfEntries: Int,
- evictInterval: JDuration,
- deleteInterval: JDuration,
- logDbCallsExceeding: FiniteDuration) {
+final class R2dbcProjectionSettings private (
+ val dialect: Dialect,
+ val schema: Option[String],
+ val offsetTable: String,
+ val timestampOffsetTable: String,
+ val managementTable: String,
+ val useConnectionFactory: String,
+ val timeWindow: JDuration,
+ val keepNumberOfEntries: Int,
+ val evictInterval: JDuration,
+ val deleteInterval: JDuration,
+ val logDbCallsExceeding: FiniteDuration
+) extends Serializable {
+
+ override def toString: String =
+ s"R2dbcProjectionSettings($dialect, $schema, $offsetTable, $timestampOffsetTable, $managementTable, " +
+ s"$useConnectionFactory, $timeWindow, $keepNumberOfEntries, $evictInterval, $deleteInterval, $logDbCallsExceeding)"
+
+ override def equals(other: Any): Boolean =
+ other match {
+ case that: R2dbcProjectionSettings =>
+ dialect == that.dialect && schema == that.schema &&
+ offsetTable == that.offsetTable && timestampOffsetTable == that.timestampOffsetTable &&
+ managementTable == that.managementTable && useConnectionFactory == that.useConnectionFactory &&
+ timeWindow == that.timeWindow && keepNumberOfEntries == that.keepNumberOfEntries &&
+ evictInterval == that.evictInterval && deleteInterval == that.deleteInterval &&
+ logDbCallsExceeding == that.logDbCallsExceeding
+ case _ => false
+ }
+
+ override def hashCode(): Int = {
+ val values = Seq(
+ dialect,
+ schema,
+ offsetTable,
+ timestampOffsetTable,
+ managementTable,
+ useConnectionFactory,
+ timeWindow,
+ keepNumberOfEntries,
+ evictInterval,
+ deleteInterval,
+ logDbCallsExceeding
+ )
+ val h = values.foldLeft(MurmurHash3.productSeed) { case (h, value) =>
+ MurmurHash3.mix(h, value.##)
+ }
+ MurmurHash3.finalizeHash(h, values.size)
+ }
+
+ private[this] def copy(
Review Comment:
Technically making `copy` private is not a source compatible change, but IIUC it is necessary for an implementation that enables keeping backwards compatibility. @pjfanning please let me know if you want this handled differently.
--
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] Projection implementation for MySQL support [pekko-persistence-r2dbc]
Posted by "Roiocam (via GitHub)" <gi...@apache.org>.
Roiocam commented on code in PR #177:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/177#discussion_r1855677696
##########
projection/src/main/scala/org/apache/pekko/projection/r2dbc/internal/mysql/MySQLR2dbcOffsetStore.scala:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pekko.projection.r2dbc.internal.mysql
+
+import java.time.Clock
+
+import org.apache.pekko
+import pekko.actor.typed.ActorSystem
+import pekko.annotation.InternalApi
+import pekko.persistence.r2dbc.internal.R2dbcExecutor
+import pekko.persistence.r2dbc.internal.Sql.DialectInterpolation
+import pekko.projection.BySlicesSourceProvider
+import pekko.projection.ProjectionId
+import pekko.projection.r2dbc.R2dbcProjectionSettings
+import pekko.projection.r2dbc.internal.R2dbcOffsetStore
+
+/**
+ * INTERNAL API
+ */
+@InternalApi
+private[projection] class MySQLR2dbcOffsetStore(
+ projectionId: ProjectionId,
+ sourceProvider: Option[BySlicesSourceProvider],
+ system: ActorSystem[_],
+ settings: R2dbcProjectionSettings,
+ r2dbcExecutor: R2dbcExecutor,
+ clock: Clock = Clock.systemUTC())
+ extends R2dbcOffsetStore(projectionId, sourceProvider, system, settings, r2dbcExecutor, clock) {
+
+ override lazy val timestampSql: String = "NOW(6)"
Review Comment:
This seems like a dialect method, can we abstract a class 'Dialect'? like Hibernate SQL Dialects does.
--
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] Projection implementation for MySQL support [pekko-persistence-r2dbc]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning merged PR #177:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/177
--
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] Projection implementation for MySQL support [pekko-persistence-r2dbc]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #177:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/177#discussion_r1848847389
##########
projection/src/main/scala/org/apache/pekko/projection/r2dbc/R2dbcProjectionSettings.scala:
##########
Review Comment:
I see the TODO on the class - so I don't mind a rewrite for 1.1.0.
It would still be nice to minimise source incompatible changes - i.e. code with against 1.0.0 would ideally still be compilable with 1.1.0.
--
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] Projection implementation for MySQL support [pekko-persistence-r2dbc]
Posted by "ptrdom (via GitHub)" <gi...@apache.org>.
ptrdom commented on code in PR #177:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/177#discussion_r1846870120
##########
projection/src/main/scala/org/apache/pekko/projection/r2dbc/R2dbcProjectionSettings.scala:
##########
Review Comment:
Since we are breaking backwards compatibility here, can we convert from `case class` to `class` too? Similar to how `R2dbcSettings` is implemented.
--
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] Projection implementation for MySQL support [pekko-persistence-r2dbc]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #177:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/177#issuecomment-2488471552
I committed a fix for the merge conflict
--
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