Re: [PR] [SpannerIO] Add low-latency configuration in Spanner Change Streams [beam]

["tianz101 (via GitHub)" <gi...@apache.org>]

tianz101 commented on code in PR #37580:
URL: https://github.com/apache/beam/pull/37580#discussion_r2861538437


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIO.java:
##########
@@ -1912,6 +1931,16 @@ public ReadChangeStream withUsingPlainTextChannel(boolean plainText) {
       return withUsingPlainTextChannel(ValueProvider.StaticValueProvider.of(plainText));
     }
 
+    /**
+     * Configures the change stream to checkpoint and flush output targeting low latency at the cost
+     * of higher rpc rate and cpu usage.
+     */
+    public ReadChangeStream withLowLatency() {
+      return toBuilder()
+          .setRealTimeCheckpointInterval(DEFAULT_LOW_LATENCY_REAL_TIME_CHECKPOINT_INTERVAL)

Review Comment:
   I understand you are lowering this to 1s to try to let the query return early so that you can do checkpoint and move forward the start ts for the next query.
   But I have seen spanner patition query start delay (time spanner really start the query vs the startTS specified in the TVF query) is about 3s. So this will really come back around 1+3=4s. During this time, if there is any heartbeat event returned, you are able to checkpoint as well. So it seems to me it is more effective to lower the heartbeam return time (100ms for example).



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/ChangeStreamsConstants.java:
##########
@@ -49,6 +49,13 @@ public class ChangeStreamsConstants {
    */
   public static final Timestamp DEFAULT_INCLUSIVE_END_AT = MAX_INCLUSIVE_END_AT;
 
+  public static final Duration DEFAULT_REAL_TIME_CHECKPOINT_INTERVAL = Duration.standardMinutes(2);
+
+  public static final int DEFAULT_HEARTBEAT_MILLIS = 2000;
+
+  public static final Duration DEFAULT_LOW_LATENCY_REAL_TIME_CHECKPOINT_INTERVAL =

Review Comment:
   nit: format to 1 or 2 lines.



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIO.java:
##########
@@ -1912,6 +1931,16 @@ public ReadChangeStream withUsingPlainTextChannel(boolean plainText) {
       return withUsingPlainTextChannel(ValueProvider.StaticValueProvider.of(plainText));
     }
 
+    /**
+     * Configures the change stream to checkpoint and flush output targeting low latency at the cost
+     * of higher rpc rate and cpu usage.
+     */
+    public ReadChangeStream withLowLatency() {

Review Comment:
   where is this function gets called?



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org