You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "sidkhillon (via GitHub)" <gi...@apache.org> on 2025/12/26 17:11:59 UTC

[PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

sidkhillon opened a new pull request, #7577:
URL: https://github.com/apache/hbase/pull/7577

   Currently, sleepForRetries (the sleep time between retry attempts during replication) is only configurable globally via the replication.source.sleepforretries configuration property. This makes it impossible to tune behavior for individual replication peers that may have different requirements.
   
   This change would add support for configuring sleepForRetries on a per-peer basis, with fallback to the global configuration when not set. It also adds UI support for displaying the field and shell support for editing the value.


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7577:
URL: https://github.com/apache/hbase/pull/7577#discussion_r2651682076


##########
hbase-client/src/main/java/org/apache/hadoop/hbase/replication/ReplicationPeerConfig.java:
##########
@@ -140,6 +142,10 @@ public boolean isSerial() {
     return serial;
   }
 

Review Comment:
   The public getter method getSleepForRetries() is missing Javadoc documentation. For consistency with other public methods in this class and to help users understand the purpose and behavior of this configuration property, add Javadoc that explains:
   - What this value represents (sleep time between retries in milliseconds)
   - The default value (0, which means use global configuration)
   - When it's used (during replication retries)
   ```suggestion
   
     /**
      * Returns the per-peer sleep time between replication retries, in milliseconds.
      * <p>
      * A value of {@code 0} means that this peer does not define its own retry sleep and the
      * global replication configuration should be used instead.
      * This value is consulted when performing replication retries for this peer.
      *
      * @return sleep time between replication retries in milliseconds, or {@code 0} to use the
      *         global configuration
      */
   ```



##########
hbase-shell/src/main/ruby/shell/commands/set_peer_sleep_for_retries.rb:
##########
@@ -0,0 +1,42 @@
+#
+# Copyright The Apache Software Foundation
+#
+# 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.
+#
+
+module Shell
+  module Commands
+    class SetPeerSleepForRetries < Command
+      def help
+        <<-EOF
+Set the replication source sleep time between retries for the specified peer.
+Examples:
+
+  # set sleep time to 2 seconds (2000ms) between retries for a peer
+  hbase> set_peer_sleep_for_retries '1', 2000
+  # unset sleep time for a peer to use the global default configured in server-side

Review Comment:
   The comment states "unset sleep time for a peer to use the global default configured in server-side" when passing 0, but this behavior differs from the bandwidth configuration pattern shown in the same file. In the bandwidth test (lines 609-622), 0 is also used as the initial/default value, but the bandwidth implementation treats 0 as "use default bandwidth". For consistency and clarity, consider documenting this explicitly in the help text that 0 has special meaning as a sentinel value for "use global default", and ensure users understand that setting it to 0 is not the same as setting it to the actual configured global default value.
   ```suggestion
   A value of 0 is treated specially as a sentinel meaning "use the global default
   sleep time configured on the server side" rather than an explicit sleep time.
   Examples:
   
     # set sleep time to 2 seconds (2000ms) between retries for a peer
     hbase> set_peer_sleep_for_retries '1', 2000
     # unset the peer-specific sleep time so that this peer uses the global default
     # configured on the server side (0 is a sentinel and does not change that default)
   ```



##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java:
##########
@@ -526,6 +522,19 @@ private long getCurrentBandwidth() {
     return peerBandwidth != 0 ? peerBandwidth : defaultBandwidth;
   }
 
+  /**
+   * Get the sleep time for retries. Check peer config first, if set use it, otherwise fall back to
+   * global configuration.
+   * @return sleep time in milliseconds
+   */

Review Comment:
   This method overrides [ReplicationSourceInterface.getSleepForRetries](1); it is advisable to add an Override annotation.
   ```suggestion
      */
     @Override
   ```



##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java:
##########
@@ -526,6 +522,19 @@ private long getCurrentBandwidth() {
     return peerBandwidth != 0 ? peerBandwidth : defaultBandwidth;
   }
 
+  /**
+   * Get the sleep time for retries. Check peer config first, if set use it, otherwise fall back to
+   * global configuration.
+   * @return sleep time in milliseconds
+   */
+  public long getSleepForRetries() {
+    long peerSleepForRetries = replicationPeer.getPeerConfig().getSleepForRetries();
+    if (peerSleepForRetries > 0) {
+      return peerSleepForRetries;
+    }
+    return this.conf.getLong("replication.source.sleepforretries", 1000);
+  }

Review Comment:
   The fallback logic is inconsistent with the existing `getCurrentBandwidth()` pattern. The bandwidth implementation (lines 519-523) treats 0 as "use default", but this implementation treats values > 0 as valid peer settings and 0 as "use default". This creates an inconsistency where setting sleepForRetries to 0 means "use global default" but for bandwidth, 0 means "use default". Consider using the same pattern as bandwidth (checking != 0 instead of > 0) for consistency, or add explicit documentation about this behavior difference.



##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java:
##########
@@ -526,6 +522,19 @@ private long getCurrentBandwidth() {
     return peerBandwidth != 0 ? peerBandwidth : defaultBandwidth;
   }
 
+  /**
+   * Get the sleep time for retries. Check peer config first, if set use it, otherwise fall back to
+   * global configuration.
+   * @return sleep time in milliseconds
+   */
+  public long getSleepForRetries() {
+    long peerSleepForRetries = replicationPeer.getPeerConfig().getSleepForRetries();
+    if (peerSleepForRetries > 0) {
+      return peerSleepForRetries;
+    }
+    return this.conf.getLong("replication.source.sleepforretries", 1000);
+  }

Review Comment:
   Missing test coverage for the new getSleepForRetries() method and its fallback logic. While the existing tests mock getSleepForRetries() to return 0L, there's no test that verifies:
   1. When peer config has a positive value (e.g., 2000), it returns that value
   2. When peer config is 0, it falls back to the global configuration value
   3. The integration between peer-level and global configuration
   
   Consider adding a dedicated unit test in TestReplicationSource.java that verifies this fallback behavior, similar to how bandwidth is tested in other parts of the codebase.



-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3694759356

   > > Since we have a Configuration Object in ReplicationPeerConfig, what about just create a combined Configuration instance and use it when creating ReplicationSource? In this way you can directly change the sleepForRetry through the configuration.
   > 
   > Just to confirm, you're suggesting I use the existing `configuration` map in ReplicationPeerConfig and store the value as "replication.source.sleepforretries". Then, we can override the config by doing something like:
   > 
   > ```java
   > Configuration combinedConf = new Configuration(globalConf);
   > // Override any values in globalConf with the existing value in peerConfig
   > peerConfig.getConfiguration().forEach(combinedConf::set);
   > // set this.conf = combinedConf in the ReplicationSource
   > ```
   > 
   > Set peer-specific override via `update_peer_config '1', CONFIG => {"replication.source.sleepforretries" => 2000}`
   > 
   > Is that the approach you are looking for?
   
   Yes.
   
   More specific, we can change the code in ReplicationSourceManager.createSource.
   
   We have a CompoundConfiguration in hbase, where we can merge multiple Configurations together. You can check ReplicationPeerConfigUtil.getPeerClusterConfiguration method to find the usage.
   
   Thanks.


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "sidkhillon (via GitHub)" <gi...@apache.org>.
sidkhillon commented on code in PR #7577:
URL: https://github.com/apache/hbase/pull/7577#discussion_r2653598626


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java:
##########
@@ -862,4 +862,14 @@ public String logPeerId() {
   public long getTotalReplicatedEdits() {
     return totalReplicatedEdits.get();
   }
+
+  @Override
+  public long getSleepForRetriesForTesting() {
+    return sleepForRetries;
+  }
+
+  @Override
+  public Map<String, ReplicationSourceShipper> getWorkerThreadsForTesting() {
+    return workerThreads;
+  }
 }

Review Comment:
   To clarify why I added it to the interface: most of the methods return ReplicationSourceInterface and therefore to use those methods in the test I'd need to either cast to the specific implementation of ReplicationSource or add the methods to the interface. I went for the latter option, but I could change it to the former.



-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "sidkhillon (via GitHub)" <gi...@apache.org>.
sidkhillon commented on code in PR #7577:
URL: https://github.com/apache/hbase/pull/7577#discussion_r2653704839


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java:
##########
@@ -862,4 +862,14 @@ public String logPeerId() {
   public long getTotalReplicatedEdits() {
     return totalReplicatedEdits.get();
   }
+
+  @Override
+  public long getSleepForRetriesForTesting() {
+    return sleepForRetries;
+  }
+
+  @Override
+  public Map<String, ReplicationSourceShipper> getWorkerThreadsForTesting() {
+    return workerThreads;
+  }
 }

Review Comment:
   Just made that fix -- thanks for the suggestion. I also realized we need to refresh the sources when we have a change in the peer config that affects the source, so I've made that change as well. Please let me know if you find any issues with it. Thanks!



-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3705065952

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 29s |  |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  2s |  |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck  |
   |||| _ Prechecks _ |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 21s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 13s |  |  master passed  |
   | +1 :green_heart: |  compile  |   2m 16s |  |  master passed  |
   | +1 :green_heart: |  javadoc  |   1m  6s |  |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 20s |  |  branch has no errors when building our shaded downstream artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 13s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 13s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   2m 16s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   2m 16s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m  5s |  |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 20s |  |  patch has no errors when building our shaded downstream artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 35s |  |  hbase-protocol-shaded in the patch passed.  |
   | +1 :green_heart: |  unit  |   1m 33s |  |  hbase-client in the patch passed.  |
   | -1 :x: |  unit  | 241m  3s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/7/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) |  hbase-server in the patch failed.  |
   | +1 :green_heart: |  unit  |   7m  4s |  |  hbase-shell in the patch passed.  |
   |  |   | 282m 59s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/7/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | javac javadoc unit compile shadedjars |
   | uname | Linux d750ffe5c636 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / d75920caa49e2ef2c4a0ea15ca8899888cc990b6 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/7/testReport/ |
   | Max. process+thread count | 4059 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/7/console |
   | versions | git=2.34.1 maven=3.9.8 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3693544062

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 27s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any anti-patterns.  |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 20s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m  3s |  |  master passed  |
   | +1 :green_heart: |  compile  |   4m 57s |  |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 30s |  |  master passed  |
   | +1 :green_heart: |  spotbugs  |   4m 29s |  |  master passed  |
   | +1 :green_heart: |  spotless  |   0m 42s |  |  branch has no errors when running spotless:check.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 11s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   2m 52s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   4m 58s |  |  the patch passed  |
   | +1 :green_heart: |  cc  |   4m 58s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   4m 58s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   1m 30s |  |  the patch passed  |
   | -0 :warning: |  rubocop  |   0m 10s | [/results-rubocop.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/3/artifact/yetus-general-check/output/results-rubocop.txt) |  The patch generated 12 new + 502 unchanged - 2 fixed = 514 total (was 504)  |
   | +1 :green_heart: |  spotbugs  |   4m 50s |  |  the patch passed  |
   | +1 :green_heart: |  hadoopcheck  |  11m  8s |  |  Patch does not cause any errors with Hadoop 3.3.6 3.4.1.  |
   | +1 :green_heart: |  hbaseprotoc  |   1m 46s |  |  the patch passed  |
   | +1 :green_heart: |  spotless  |   0m 42s |  |  patch has no errors when running spotless:check.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 35s |  |  The patch does not generate ASF License warnings.  |
   |  |   |  51m 54s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/3/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless cc buflint bufcompat hbaseprotoc rubocop |
   | uname | Linux 4b57a8ef0ca5 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / cae2efb8069d6bb6a5cdf96d0656c4d72bfb290c |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | Max. process+thread count | 85 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/3/console |
   | versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 rubocop=1.37.1 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3693676313

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 29s |  |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  2s |  |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck  |
   |||| _ Prechecks _ |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 21s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 23s |  |  master passed  |
   | +1 :green_heart: |  compile  |   2m 14s |  |  master passed  |
   | +1 :green_heart: |  javadoc  |   1m  5s |  |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 15s |  |  branch has no errors when building our shaded downstream artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 13s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 10s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   2m 14s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   2m 14s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m  5s |  |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 18s |  |  patch has no errors when building our shaded downstream artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 34s |  |  hbase-protocol-shaded in the patch passed.  |
   | +1 :green_heart: |  unit  |   1m 32s |  |  hbase-client in the patch passed.  |
   | +1 :green_heart: |  unit  | 227m 33s |  |  hbase-server in the patch passed.  |
   | +1 :green_heart: |  unit  |   7m 19s |  |  hbase-shell in the patch passed.  |
   |  |   | 268m 59s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/3/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | javac javadoc unit compile shadedjars |
   | uname | Linux 6f712e9d0480 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / cae2efb8069d6bb6a5cdf96d0656c4d72bfb290c |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/3/testReport/ |
   | Max. process+thread count | 3729 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/3/console |
   | versions | git=2.34.1 maven=3.9.8 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "taklwu (via GitHub)" <gi...@apache.org>.
taklwu commented on code in PR #7577:
URL: https://github.com/apache/hbase/pull/7577#discussion_r2653578912


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java:
##########
@@ -862,4 +862,14 @@ public String logPeerId() {
   public long getTotalReplicatedEdits() {
     return totalReplicatedEdits.get();
   }
+
+  @Override
+  public long getSleepForRetriesForTesting() {
+    return sleepForRetries;
+  }
+
+  @Override
+  public Map<String, ReplicationSourceShipper> getWorkerThreadsForTesting() {
+    return workerThreads;
+  }
 }

Review Comment:
   nit: I don't know if we should have the `*ForTesting` in any interface in the scope of the IA.Private, but at least to align with other lines, we can keep the `getSleepForRetries` (no `ForTesting`) and only have the `getWorkerThreadsForTesting` in the `ReplicationSource`.
   
   ```suggestion
   
     @Override
     public long getSleepForRetries() {
       return sleepForRetries;
     }
   }
   ```



-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "charlesconnell (via GitHub)" <gi...@apache.org>.
charlesconnell merged PR #7577:
URL: https://github.com/apache/hbase/pull/7577


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "sidkhillon (via GitHub)" <gi...@apache.org>.
sidkhillon commented on code in PR #7577:
URL: https://github.com/apache/hbase/pull/7577#discussion_r2653571726


##########
hbase-server/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java:
##########
@@ -373,4 +374,52 @@ public void testRemoveRemoteWALs() throws Exception {
     manager.cleanOldLogs(walName, true, source);
     assertFalse(FS.exists(remoteWAL));
   }
+
+  @Test
+  public void testPeerConfigurationOverridesPropagate() throws Exception {
+    Configuration globalConf = UTIL.getConfiguration();
+    long globalSleepValue = 1000L;
+    globalConf.setLong("replication.source.sleepforretries", globalSleepValue);

Review Comment:
   Sure, just added that new 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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "taklwu (via GitHub)" <gi...@apache.org>.
taklwu commented on code in PR #7577:
URL: https://github.com/apache/hbase/pull/7577#discussion_r2653554994


##########
hbase-server/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java:
##########
@@ -373,4 +374,52 @@ public void testRemoveRemoteWALs() throws Exception {
     manager.cleanOldLogs(walName, true, source);
     assertFalse(FS.exists(remoteWAL));
   }
+
+  @Test
+  public void testPeerConfigurationOverridesPropagate() throws Exception {
+    Configuration globalConf = UTIL.getConfiguration();
+    long globalSleepValue = 1000L;
+    globalConf.setLong("replication.source.sleepforretries", globalSleepValue);

Review Comment:
   nit: is that possible to add a new assertion or a new test for another peer (two peers) is only using this `globalSleepValue`? 



-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3700715413

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 31s |  |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck  |
   |||| _ Prechecks _ |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 34s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 37s |  |  master passed  |
   | +1 :green_heart: |  compile  |   3m  4s |  |  master passed  |
   | +1 :green_heart: |  javadoc  |   1m 30s |  |  master passed  |
   | +1 :green_heart: |  shadedjars  |   7m 15s |  |  branch has no errors when building our shaded downstream artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 19s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   2m 44s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   2m 44s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m 17s |  |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 56s |  |  patch has no errors when building our shaded downstream artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 39s |  |  hbase-protocol-shaded in the patch passed.  |
   | +1 :green_heart: |  unit  |   1m 56s |  |  hbase-client in the patch passed.  |
   | -1 :x: |  unit  | 290m 11s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/4/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) |  hbase-server in the patch failed.  |
   | +1 :green_heart: |  unit  |   7m 51s |  |  hbase-shell in the patch passed.  |
   |  |   | 339m 44s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/4/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | javac javadoc unit compile shadedjars |
   | uname | Linux 83d823a81fcc 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 49f6d5240bea05ac4f8149b8eba1c1dafae611a8 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/4/testReport/ |
   | Max. process+thread count | 3482 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/4/console |
   | versions | git=2.34.1 maven=3.9.8 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "sidkhillon (via GitHub)" <gi...@apache.org>.
sidkhillon commented on code in PR #7577:
URL: https://github.com/apache/hbase/pull/7577#discussion_r2653587588


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java:
##########
@@ -862,4 +862,14 @@ public String logPeerId() {
   public long getTotalReplicatedEdits() {
     return totalReplicatedEdits.get();
   }
+
+  @Override
+  public long getSleepForRetriesForTesting() {
+    return sleepForRetries;
+  }
+
+  @Override
+  public Map<String, ReplicationSourceShipper> getWorkerThreadsForTesting() {
+    return workerThreads;
+  }
 }

Review Comment:
   Just to clarify here, are you looking to just remove `getWorkerThreadsForTesting` from the interface? We'd still need the functionality here to be able to do the unit tests.
   
   I put up a change with what I interpreted this as. Please let me know if that's what you're looking for.



-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3711214850

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 29s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any anti-patterns.  |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 35s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 20s |  |  master passed  |
   | +1 :green_heart: |  compile  |   4m 59s |  |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 31s |  |  master passed  |
   | +1 :green_heart: |  spotbugs  |   4m 35s |  |  master passed  |
   | +1 :green_heart: |  spotless  |   0m 46s |  |  branch has no errors when running spotless:check.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   2m 54s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   4m 53s |  |  the patch passed  |
   | +1 :green_heart: |  cc  |   4m 53s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   4m 53s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   1m 29s |  |  the patch passed  |
   | -0 :warning: |  rubocop  |   0m 19s | [/results-rubocop.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/9/artifact/yetus-general-check/output/results-rubocop.txt) |  The patch generated 3 new + 504 unchanged - 0 fixed = 507 total (was 504)  |
   | +1 :green_heart: |  spotbugs  |   4m 47s |  |  the patch passed  |
   | +1 :green_heart: |  hadoopcheck  |  11m 21s |  |  Patch does not cause any errors with Hadoop 3.3.6 3.4.1.  |
   | +1 :green_heart: |  hbaseprotoc  |   1m 44s |  |  the patch passed  |
   | +1 :green_heart: |  spotless  |   0m 43s |  |  patch has no errors when running spotless:check.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 36s |  |  The patch does not generate ASF License warnings.  |
   |  |   |  53m 15s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/9/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless cc buflint bufcompat hbaseprotoc rubocop |
   | uname | Linux cc539b95e695 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 2d412fe6078efe32b3ce93dbc9c1a49857f9c147 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | Max. process+thread count | 86 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/9/console |
   | versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 rubocop=1.37.1 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "sidkhillon (via GitHub)" <gi...@apache.org>.
sidkhillon commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3712204959

   The test that is failing seems unrelated to my changes and is just timing out. I have re-run it locally and that test passes as well. The only changes since the test last passed was [d75920c](https://github.com/apache/hbase/pull/7577/commits/d75920caa49e2ef2c4a0ea15ca8899888cc990b6). I am kicking off another build, but I think that test might just be flakey.


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3701494088

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 10s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any anti-patterns.  |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   2m 32s |  |  master passed  |
   | +1 :green_heart: |  compile  |   3m 47s |  |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m  8s |  |  master passed  |
   | +1 :green_heart: |  spotbugs  |   3m 22s |  |  master passed  |
   | +1 :green_heart: |  spotless  |   0m 39s |  |  branch has no errors when running spotless:check.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 10s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   2m 12s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   3m 45s |  |  the patch passed  |
   | +1 :green_heart: |  cc  |   3m 45s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   3m 45s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   1m  7s |  |  the patch passed  |
   | -0 :warning: |  rubocop  |   0m  7s | [/results-rubocop.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/6/artifact/yetus-general-check/output/results-rubocop.txt) |  The patch generated 3 new + 504 unchanged - 0 fixed = 507 total (was 504)  |
   | +1 :green_heart: |  spotbugs  |   3m 35s |  |  the patch passed  |
   | +1 :green_heart: |  hadoopcheck  |   8m 25s |  |  Patch does not cause any errors with Hadoop 3.3.6 3.4.1.  |
   | +1 :green_heart: |  hbaseprotoc  |   1m 14s |  |  the patch passed  |
   | +1 :green_heart: |  spotless  |   0m 32s |  |  patch has no errors when running spotless:check.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 27s |  |  The patch does not generate ASF License warnings.  |
   |  |   |  38m 59s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.48 ServerAPI=1.48 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/6/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless cc buflint bufcompat hbaseprotoc rubocop |
   | uname | Linux badf1eb8d21e 6.8.0-1024-aws #26~22.04.1-Ubuntu SMP Wed Feb 19 06:54:57 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 851c7373c91687de2dc8e9322993c4229c6cc3d7 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | Max. process+thread count | 90 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/6/console |
   | versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 rubocop=1.37.1 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "sidkhillon (via GitHub)" <gi...@apache.org>.
sidkhillon commented on code in PR #7577:
URL: https://github.com/apache/hbase/pull/7577#discussion_r2653837299


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/PeerProcedureHandlerImpl.java:
##########
@@ -108,6 +109,33 @@ public void disablePeer(String peerId) throws ReplicationException, IOException
     refreshPeerState(peerId);
   }
 
+  private boolean hasReplicationConfigChange(ReplicationPeerConfig oldConfig,
+    ReplicationPeerConfig newConfig) {
+    Map<String, String> oldReplicationConfigs = oldConfig.getConfiguration();
+    Map<String, String> newReplicationConfigs = newConfig.getConfiguration();
+
+    // Check if any replication.source.* keys have changed values
+    for (Map.Entry<String, String> entry : newReplicationConfigs.entrySet()) {
+      String key = entry.getKey();
+      if (key.startsWith("replication.source.")) {

Review Comment:
   The first two couldn't be used on a per-peer basis while the last is only used by the InterClusterReplicationEndpoint and not a ReplicationSource. Therefore, I think it is fine to not include those.



-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3693175409

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 28s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any anti-patterns.  |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 19s |  |  master passed  |
   | +1 :green_heart: |  compile  |   5m  2s |  |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 33s |  |  master passed  |
   | +1 :green_heart: |  spotbugs  |   4m 29s |  |  master passed  |
   | +1 :green_heart: |  spotless  |   0m 48s |  |  branch has no errors when running spotless:check.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  |  Maven dependency ordering for patch  |
   | -1 :x: |  mvninstall  |   1m 20s | [/patch-mvninstall-root.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/1/artifact/yetus-general-check/output/patch-mvninstall-root.txt) |  root in the patch failed.  |
   | -1 :x: |  compile  |   0m 37s | [/patch-compile-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/1/artifact/yetus-general-check/output/patch-compile-hbase-server.txt) |  hbase-server in the patch failed.  |
   | -1 :x: |  cc  |   0m 37s | [/patch-compile-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/1/artifact/yetus-general-check/output/patch-compile-hbase-server.txt) |  hbase-server in the patch failed.  |
   | -0 :warning: |  javac  |   0m 37s | [/patch-compile-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/1/artifact/yetus-general-check/output/patch-compile-hbase-server.txt) |  hbase-server in the patch failed.  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   1m 22s |  |  the patch passed  |
   | -0 :warning: |  rubocop  |   0m 16s | [/results-rubocop.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/1/artifact/yetus-general-check/output/results-rubocop.txt) |  The patch generated 12 new + 502 unchanged - 2 fixed = 514 total (was 504)  |
   | -1 :x: |  spotbugs  |   0m 37s | [/patch-spotbugs-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/1/artifact/yetus-general-check/output/patch-spotbugs-hbase-server.txt) |  hbase-server in the patch failed.  |
   | -1 :x: |  hadoopcheck  |   1m 41s |  |  The patch causes 18 errors with Hadoop v3.3.6.  |
   | -1 :x: |  hadoopcheck  |   3m 22s |  |  The patch causes 18 errors with Hadoop v3.4.1.  |
   | -1 :x: |  hbaseprotoc  |   0m 33s | [/patch-hbaseprotoc-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/1/artifact/yetus-general-check/output/patch-hbaseprotoc-hbase-server.txt) |  hbase-server in the patch failed.  |
   | +1 :green_heart: |  spotless  |   0m 42s |  |  patch has no errors when running spotless:check.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 31s |  |  The patch does not generate ASF License warnings.  |
   |  |   |  33m 15s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless cc buflint bufcompat hbaseprotoc rubocop |
   | uname | Linux 92e401e2aebc 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / e40230a65da2e89a910a023c9241c4b3a636ec74 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | hadoopcheck | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/1/artifact/yetus-general-check/output/patch-javac-3.3.6.txt |
   | hadoopcheck | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/1/artifact/yetus-general-check/output/patch-javac-3.4.1.txt |
   | Max. process+thread count | 85 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/1/console |
   | versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 rubocop=1.37.1 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3693245696

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 27s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any anti-patterns.  |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 11s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   2m 59s |  |  master passed  |
   | +1 :green_heart: |  compile  |   4m 58s |  |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 29s |  |  master passed  |
   | +1 :green_heart: |  spotbugs  |   4m 27s |  |  master passed  |
   | +1 :green_heart: |  spotless  |   0m 43s |  |  branch has no errors when running spotless:check.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   2m 59s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   4m 47s |  |  the patch passed  |
   | +1 :green_heart: |  cc  |   4m 47s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   4m 47s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   1m 27s |  |  the patch passed  |
   | -0 :warning: |  rubocop  |   0m 10s | [/results-rubocop.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/2/artifact/yetus-general-check/output/results-rubocop.txt) |  The patch generated 12 new + 502 unchanged - 2 fixed = 514 total (was 504)  |
   | +1 :green_heart: |  spotbugs  |   4m 40s |  |  the patch passed  |
   | +1 :green_heart: |  hadoopcheck  |  10m 42s |  |  Patch does not cause any errors with Hadoop 3.3.6 3.4.1.  |
   | +1 :green_heart: |  hbaseprotoc  |   1m 46s |  |  the patch passed  |
   | +1 :green_heart: |  spotless  |   0m 42s |  |  patch has no errors when running spotless:check.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 35s |  |  The patch does not generate ASF License warnings.  |
   |  |   |  50m 47s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless cc buflint bufcompat hbaseprotoc rubocop |
   | uname | Linux 9fd505385b64 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 251d4f08c3a85474b75f0bf9619dbd928da1deed |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | Max. process+thread count | 85 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/2/console |
   | versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 rubocop=1.37.1 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3693582084

   Since we have a Configuration Object in ReplicationPeerConfig, what about just create a combined Configuration instance and use it when creating ReplicationSource? In this way you can directly change the sleepForRetry through the configuration.


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3712884696

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 27s |  |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck  |
   |||| _ Prechecks _ |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 34s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 34s |  |  master passed  |
   | +1 :green_heart: |  compile  |   2m 14s |  |  master passed  |
   | +1 :green_heart: |  javadoc  |   1m  6s |  |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 22s |  |  branch has no errors when building our shaded downstream artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 13s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 12s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   2m 15s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   2m 15s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m  6s |  |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 22s |  |  patch has no errors when building our shaded downstream artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 35s |  |  hbase-protocol-shaded in the patch passed.  |
   | +1 :green_heart: |  unit  |   1m 33s |  |  hbase-client in the patch passed.  |
   | -1 :x: |  unit  | 238m 49s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/10/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) |  hbase-server in the patch failed.  |
   | +1 :green_heart: |  unit  |   7m  1s |  |  hbase-shell in the patch passed.  |
   |  |   | 280m 44s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/10/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | javac javadoc unit compile shadedjars |
   | uname | Linux e666c3380011 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 46b153346aa73a6d9c0605a931513f6e5a6d447c |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/10/testReport/ |
   | Max. process+thread count | 4684 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/10/console |
   | versions | git=2.34.1 maven=3.9.8 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3700074145

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 33s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any anti-patterns.  |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 39s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 21s |  |  master passed  |
   | +1 :green_heart: |  compile  |   5m  3s |  |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 33s |  |  master passed  |
   | +1 :green_heart: |  spotbugs  |   4m 32s |  |  master passed  |
   | +1 :green_heart: |  spotless  |   0m 46s |  |  branch has no errors when running spotless:check.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 13s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   2m 52s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   5m  0s |  |  the patch passed  |
   | +1 :green_heart: |  cc  |   5m  0s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   5m  0s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   1m 32s |  |  the patch passed  |
   | -0 :warning: |  rubocop  |   0m 10s | [/results-rubocop.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/4/artifact/yetus-general-check/output/results-rubocop.txt) |  The patch generated 3 new + 504 unchanged - 0 fixed = 507 total (was 504)  |
   | +1 :green_heart: |  spotbugs  |   4m 52s |  |  the patch passed  |
   | +1 :green_heart: |  hadoopcheck  |  11m 17s |  |  Patch does not cause any errors with Hadoop 3.3.6 3.4.1.  |
   | +1 :green_heart: |  hbaseprotoc  |   1m 44s |  |  the patch passed  |
   | +1 :green_heart: |  spotless  |   0m 43s |  |  patch has no errors when running spotless:check.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 36s |  |  The patch does not generate ASF License warnings.  |
   |  |   |  53m  5s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/4/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless cc buflint bufcompat hbaseprotoc rubocop |
   | uname | Linux 7e93c6cea647 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 49f6d5240bea05ac4f8149b8eba1c1dafae611a8 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | Max. process+thread count | 85 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/4/console |
   | versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 rubocop=1.37.1 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "taklwu (via GitHub)" <gi...@apache.org>.
taklwu commented on code in PR #7577:
URL: https://github.com/apache/hbase/pull/7577#discussion_r2653616195


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java:
##########
@@ -862,4 +862,14 @@ public String logPeerId() {
   public long getTotalReplicatedEdits() {
     return totalReplicatedEdits.get();
   }
+
+  @Override
+  public long getSleepForRetriesForTesting() {
+    return sleepForRetries;
+  }
+
+  @Override
+  public Map<String, ReplicationSourceShipper> getWorkerThreadsForTesting() {
+    return workerThreads;
+  }
 }

Review Comment:
   yeah, it's mostly related to `manager.getSources()` were returning a list of `ReplicationSourceInterface`. 
   
   using cast was what I was thinking like below , and you got my point as well.
   
   ```
       Map<String, ReplicationSourceShipper> workers =
         ((ReplicationSource) source).getWorkerThreadsForTesting();
   ```



-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "sidkhillon (via GitHub)" <gi...@apache.org>.
sidkhillon commented on code in PR #7577:
URL: https://github.com/apache/hbase/pull/7577#discussion_r2653598626


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java:
##########
@@ -862,4 +862,14 @@ public String logPeerId() {
   public long getTotalReplicatedEdits() {
     return totalReplicatedEdits.get();
   }
+
+  @Override
+  public long getSleepForRetriesForTesting() {
+    return sleepForRetries;
+  }
+
+  @Override
+  public Map<String, ReplicationSourceShipper> getWorkerThreadsForTesting() {
+    return workerThreads;
+  }
 }

Review Comment:
   To clarify why I added it to the interface: most of the methods return ReplicationSourceInterface and therefore to use those methods in the test I'd need to either cast to the specific implementation of ReplicationSource or add the methods to the interface. I went for the latter option, but I could change it to the former if you'd prefer.



-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3705716982

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 29s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any anti-patterns.  |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   2m 55s |  |  master passed  |
   | +1 :green_heart: |  compile  |   4m 57s |  |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 30s |  |  master passed  |
   | +1 :green_heart: |  spotbugs  |   4m 28s |  |  master passed  |
   | +1 :green_heart: |  spotless  |   0m 43s |  |  branch has no errors when running spotless:check.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 11s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   2m 51s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   4m 59s |  |  the patch passed  |
   | +1 :green_heart: |  cc  |   4m 59s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   4m 59s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  1s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   1m 31s |  |  the patch passed  |
   | -0 :warning: |  rubocop  |   0m 11s | [/results-rubocop.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/8/artifact/yetus-general-check/output/results-rubocop.txt) |  The patch generated 3 new + 504 unchanged - 0 fixed = 507 total (was 504)  |
   | +1 :green_heart: |  spotbugs  |   4m 50s |  |  the patch passed  |
   | +1 :green_heart: |  hadoopcheck  |  11m  1s |  |  Patch does not cause any errors with Hadoop 3.3.6 3.4.1.  |
   | +1 :green_heart: |  hbaseprotoc  |   1m 46s |  |  the patch passed  |
   | +1 :green_heart: |  spotless  |   0m 43s |  |  patch has no errors when running spotless:check.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 35s |  |  The patch does not generate ASF License warnings.  |
   |  |   |  51m 32s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/8/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless cc buflint bufcompat hbaseprotoc rubocop |
   | uname | Linux e556c627eecd 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 13b7661dd5a48eac2a1ac685a9b373a9296d210a |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | Max. process+thread count | 85 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/8/console |
   | versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 rubocop=1.37.1 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3701813760

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   1m  7s |  |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck  |
   |||| _ Prechecks _ |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 44s |  |  master passed  |
   | +1 :green_heart: |  compile  |   2m 24s |  |  master passed  |
   | +1 :green_heart: |  javadoc  |   1m  7s |  |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 20s |  |  branch has no errors when building our shaded downstream artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 12s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   2m 15s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   2m 15s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m  5s |  |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 20s |  |  patch has no errors when building our shaded downstream artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 35s |  |  hbase-protocol-shaded in the patch passed.  |
   | +1 :green_heart: |  unit  |   1m 32s |  |  hbase-client in the patch passed.  |
   | +1 :green_heart: |  unit  | 218m 17s |  |  hbase-server in the patch passed.  |
   | +1 :green_heart: |  unit  |   7m  1s |  |  hbase-shell in the patch passed.  |
   |  |   | 260m 41s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/6/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | javac javadoc unit compile shadedjars |
   | uname | Linux c02cb19ef657 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 851c7373c91687de2dc8e9322993c4229c6cc3d7 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/6/testReport/ |
   | Max. process+thread count | 4308 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/6/console |
   | versions | git=2.34.1 maven=3.9.8 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "sidkhillon (via GitHub)" <gi...@apache.org>.
sidkhillon commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3704545052

   Thank you for the review. I just put up a commit addressing those 4 issues in both this PR and in #7578. Please let me know if those changes work for you.


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3712084580

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 42s |  |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  4s |  |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck  |
   |||| _ Prechecks _ |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 34s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   5m 24s |  |  master passed  |
   | +1 :green_heart: |  compile  |   3m 50s |  |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m  3s |  |  master passed  |
   | +1 :green_heart: |  shadedjars  |   9m  7s |  |  branch has no errors when building our shaded downstream artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 22s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 51s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   3m 37s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   3m 37s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m 22s |  |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 51s |  |  patch has no errors when building our shaded downstream artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m  1s |  |  hbase-protocol-shaded in the patch passed.  |
   | +1 :green_heart: |  unit  |   2m 43s |  |  hbase-client in the patch passed.  |
   | -1 :x: |  unit  | 262m 34s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/9/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) |  hbase-server in the patch failed.  |
   | +1 :green_heart: |  unit  |   7m  2s |  |  hbase-shell in the patch passed.  |
   |  |   | 319m 34s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/9/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | javac javadoc unit compile shadedjars |
   | uname | Linux 675c166a222f 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 2d412fe6078efe32b3ce93dbc9c1a49857f9c147 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/9/testReport/ |
   | Max. process+thread count | 5347 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/9/console |
   | versions | git=2.34.1 maven=3.9.8 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "taklwu (via GitHub)" <gi...@apache.org>.
taklwu commented on code in PR #7577:
URL: https://github.com/apache/hbase/pull/7577#discussion_r2651760363


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java:
##########
@@ -204,8 +202,6 @@ public void init(Configuration conf, FileSystem fs, ReplicationSourceManager man
     this.waitOnEndpointSeconds =
       this.conf.getInt(WAIT_ON_ENDPOINT_SECONDS, DEFAULT_WAIT_ON_ENDPOINT_SECONDS);
     decorateConf();
-    // 1 second
-    this.sleepForRetries = this.conf.getLong("replication.source.sleepforretries", 1000);

Review Comment:
   nit: do you think this `sleepForRetries` would be changing dynamically after the initialization ? IIRC it would be only reloading via `refreshSources` (e.g. via `updateReplicationPeerConfig` ) such that this value would only created once per peer configuration or refresh. 
   
   so, can we keep this variable and just call `getSleepForRetries()` once within `ReplicationSource.java`?



-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3701435952

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 44s |  |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  4s |  |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck  |
   |||| _ Prechecks _ |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 30s |  |  master passed  |
   | +1 :green_heart: |  compile  |   2m 51s |  |  master passed  |
   | +1 :green_heart: |  javadoc  |   1m 32s |  |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m 42s |  |  branch has no errors when building our shaded downstream artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 20s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   3m 14s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   3m 14s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m 39s |  |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 52s |  |  patch has no errors when building our shaded downstream artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 51s |  |  hbase-protocol-shaded in the patch passed.  |
   | +1 :green_heart: |  unit  |   2m 10s |  |  hbase-client in the patch passed.  |
   | -1 :x: |  unit  | 308m 10s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/5/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) |  hbase-server in the patch failed.  |
   | +1 :green_heart: |  unit  |   8m 49s |  |  hbase-shell in the patch passed.  |
   |  |   | 361m 51s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/5/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | javac javadoc unit compile shadedjars |
   | uname | Linux 0686d93ef510 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 72e9655756809634724de8123fb9f825f40460b8 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/5/testReport/ |
   | Max. process+thread count | 3428 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/5/console |
   | versions | git=2.34.1 maven=3.9.8 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "sidkhillon (via GitHub)" <gi...@apache.org>.
sidkhillon commented on code in PR #7577:
URL: https://github.com/apache/hbase/pull/7577#discussion_r2653837299


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/PeerProcedureHandlerImpl.java:
##########
@@ -108,6 +109,33 @@ public void disablePeer(String peerId) throws ReplicationException, IOException
     refreshPeerState(peerId);
   }
 
+  private boolean hasReplicationConfigChange(ReplicationPeerConfig oldConfig,
+    ReplicationPeerConfig newConfig) {
+    Map<String, String> oldReplicationConfigs = oldConfig.getConfiguration();
+    Map<String, String> newReplicationConfigs = newConfig.getConfiguration();
+
+    // Check if any replication.source.* keys have changed values
+    for (Map.Entry<String, String> entry : newReplicationConfigs.entrySet()) {
+      String key = entry.getKey();
+      if (key.startsWith("replication.source.")) {

Review Comment:
   As far as I can tell, the first two couldn't be used on a per-peer basis while the last is only used by the InterClusterReplicationEndpoint and not a ReplicationSource. Therefore, I think it is fine to not include those.



-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "sidkhillon (via GitHub)" <gi...@apache.org>.
sidkhillon commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3700475541

   I have updated https://github.com/apache/hbase/pull/7578 to reflect the changes suggested by PR comments as well. It only has minor differences from this branch.


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on code in PR #7577:
URL: https://github.com/apache/hbase/pull/7577#discussion_r2656186984


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java:
##########
@@ -862,4 +862,12 @@ public String logPeerId() {
   public long getTotalReplicatedEdits() {
     return totalReplicatedEdits.get();
   }
+
+  public long getSleepForRetries() {

Review Comment:
   Does this method need to be public?



##########
hbase-server/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestPeerProcedureHandlerImpl.java:
##########
@@ -0,0 +1,135 @@
+/*
+ * 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.hadoop.hbase.replication.regionserver;
+
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.replication.ReplicationPeer.PeerState;
+import org.apache.hadoop.hbase.replication.ReplicationPeerConfig;
+import org.apache.hadoop.hbase.replication.ReplicationPeerImpl;
+import org.apache.hadoop.hbase.replication.ReplicationPeers;
+import org.apache.hadoop.hbase.testclassification.ReplicationTests;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category({ ReplicationTests.class, SmallTests.class })

Review Comment:
   Let's use junit5 for new test.



##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java:
##########
@@ -862,4 +862,12 @@ public String logPeerId() {
   public long getTotalReplicatedEdits() {
     return totalReplicatedEdits.get();
   }
+
+  public long getSleepForRetries() {
+    return sleepForRetries;
+  }
+
+  public Map<String, ReplicationSourceShipper> getWorkerThreadsForTesting() {

Review Comment:
   workerThreads is protected, which can be accessed directly in tests in the same package.



##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceShipper.java:
##########
@@ -355,4 +355,12 @@ void clearWALEntryBatch() {
         totalReleasedBytes);
     }
   }
+
+  long getSleepForRetries() {
+    return sleepForRetries;
+  }
+
+  ReplicationSourceWALReader getEntryReaderForTesting() {

Review Comment:
   entryReader is already package private, we can access it directly in 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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3706152365

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 30s |  |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck  |
   |||| _ Prechecks _ |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 28s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 24s |  |  master passed  |
   | +1 :green_heart: |  compile  |   2m 14s |  |  master passed  |
   | +1 :green_heart: |  javadoc  |   1m  5s |  |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 19s |  |  branch has no errors when building our shaded downstream artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 13s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   2m 16s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   2m 16s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m  5s |  |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 23s |  |  patch has no errors when building our shaded downstream artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 35s |  |  hbase-protocol-shaded in the patch passed.  |
   | +1 :green_heart: |  unit  |   1m 36s |  |  hbase-client in the patch passed.  |
   | -1 :x: |  unit  | 239m 44s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/8/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) |  hbase-server in the patch failed.  |
   | +1 :green_heart: |  unit  |   6m 53s |  |  hbase-shell in the patch passed.  |
   |  |   | 281m 26s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/8/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | javac javadoc unit compile shadedjars |
   | uname | Linux d252045e2c0f 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 13b7661dd5a48eac2a1ac685a9b373a9296d210a |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/8/testReport/ |
   | Max. process+thread count | 4512 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/8/console |
   | versions | git=2.34.1 maven=3.9.8 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3693176721

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 34s |  |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck  |
   |||| _ Prechecks _ |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 35s |  |  master passed  |
   | +1 :green_heart: |  compile  |   2m 16s |  |  master passed  |
   | +1 :green_heart: |  javadoc  |   1m  8s |  |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 22s |  |  branch has no errors when building our shaded downstream artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  |  Maven dependency ordering for patch  |
   | -1 :x: |  mvninstall  |   1m 29s | [/patch-mvninstall-root.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/1/artifact/yetus-jdk17-hadoop3-check/output/patch-mvninstall-root.txt) |  root in the patch failed.  |
   | -1 :x: |  compile  |   0m 43s | [/patch-compile-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/1/artifact/yetus-jdk17-hadoop3-check/output/patch-compile-hbase-server.txt) |  hbase-server in the patch failed.  |
   | -0 :warning: |  javac  |   0m 43s | [/patch-compile-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/1/artifact/yetus-jdk17-hadoop3-check/output/patch-compile-hbase-server.txt) |  hbase-server in the patch failed.  |
   | +1 :green_heart: |  javadoc  |   1m  5s |  |  the patch passed  |
   | -1 :x: |  shadedjars  |   4m 31s |  |  patch has 18 errors when building our shaded downstream artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 34s |  |  hbase-protocol-shaded in the patch passed.  |
   | +1 :green_heart: |  unit  |   1m 34s |  |  hbase-client in the patch passed.  |
   | -1 :x: |  unit  |   0m 43s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/1/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) |  hbase-server in the patch failed.  |
   | +1 :green_heart: |  unit  |   6m 44s |  |  hbase-shell in the patch passed.  |
   |  |   |  34m 31s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/1/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | javac javadoc unit compile shadedjars |
   | uname | Linux 80f66f9d2011 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / e40230a65da2e89a910a023c9241c4b3a636ec74 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | shadedjars | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/1/artifact/yetus-jdk17-hadoop3-check/output/patch-shadedjars.txt |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/1/testReport/ |
   | Max. process+thread count | 1594 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/1/console |
   | versions | git=2.34.1 maven=3.9.8 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3704625214

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 29s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any anti-patterns.  |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 22s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   2m 51s |  |  master passed  |
   | +1 :green_heart: |  compile  |   4m 57s |  |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 30s |  |  master passed  |
   | +1 :green_heart: |  spotbugs  |   4m 27s |  |  master passed  |
   | +1 :green_heart: |  spotless  |   0m 43s |  |  branch has no errors when running spotless:check.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   2m 52s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   4m 54s |  |  the patch passed  |
   | +1 :green_heart: |  cc  |   4m 54s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   4m 54s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   1m 32s |  |  the patch passed  |
   | -0 :warning: |  rubocop  |   0m 11s | [/results-rubocop.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/7/artifact/yetus-general-check/output/results-rubocop.txt) |  The patch generated 3 new + 504 unchanged - 0 fixed = 507 total (was 504)  |
   | +1 :green_heart: |  spotbugs  |   4m 49s |  |  the patch passed  |
   | +1 :green_heart: |  hadoopcheck  |  11m  9s |  |  Patch does not cause any errors with Hadoop 3.3.6 3.4.1.  |
   | +1 :green_heart: |  hbaseprotoc  |   1m 44s |  |  the patch passed  |
   | +1 :green_heart: |  spotless  |   0m 42s |  |  patch has no errors when running spotless:check.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 35s |  |  The patch does not generate ASF License warnings.  |
   |  |   |  51m 51s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/7/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless cc buflint bufcompat hbaseprotoc rubocop |
   | uname | Linux bba9ed5aa5ea 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / d75920caa49e2ef2c4a0ea15ca8899888cc990b6 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | Max. process+thread count | 84 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/7/console |
   | versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 rubocop=1.37.1 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3693429977

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 29s |  |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck  |
   |||| _ Prechecks _ |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 10s |  |  master passed  |
   | +1 :green_heart: |  compile  |   2m 13s |  |  master passed  |
   | +1 :green_heart: |  javadoc  |   1m  4s |  |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 17s |  |  branch has no errors when building our shaded downstream artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 13s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 15s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   2m 14s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   2m 14s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m  5s |  |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 16s |  |  patch has no errors when building our shaded downstream artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 34s |  |  hbase-protocol-shaded in the patch passed.  |
   | +1 :green_heart: |  unit  |   1m 31s |  |  hbase-client in the patch passed.  |
   | -1 :x: |  unit  | 214m 48s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/2/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) |  hbase-server in the patch failed.  |
   | +1 :green_heart: |  unit  |   7m 10s |  |  hbase-shell in the patch passed.  |
   |  |   | 256m  8s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/2/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | javac javadoc unit compile shadedjars |
   | uname | Linux e1d3e786404c 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 251d4f08c3a85474b75f0bf9619dbd928da1deed |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/2/testReport/ |
   | Max. process+thread count | 4465 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/2/console |
   | versions | git=2.34.1 maven=3.9.8 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "taklwu (via GitHub)" <gi...@apache.org>.
taklwu commented on code in PR #7577:
URL: https://github.com/apache/hbase/pull/7577#discussion_r2653616195


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java:
##########
@@ -862,4 +862,14 @@ public String logPeerId() {
   public long getTotalReplicatedEdits() {
     return totalReplicatedEdits.get();
   }
+
+  @Override
+  public long getSleepForRetriesForTesting() {
+    return sleepForRetries;
+  }
+
+  @Override
+  public Map<String, ReplicationSourceShipper> getWorkerThreadsForTesting() {
+    return workerThreads;
+  }
 }

Review Comment:
   yeah, it's mostly related to `manager.getSources()` were returning a list of `ReplicationSourceInterface`. 
   
   using cast was what I was thinking like below , and you got my point as well.
   
   ```
   Map<String, ReplicationSourceShipper> workers = (ReplicationSource) source.getWorkerThreadsForTesting();
   ```



-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3712360339

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 28s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any anti-patterns.  |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   2m 54s |  |  master passed  |
   | +1 :green_heart: |  compile  |   4m 58s |  |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 29s |  |  master passed  |
   | +1 :green_heart: |  spotbugs  |   4m 27s |  |  master passed  |
   | +1 :green_heart: |  spotless  |   0m 43s |  |  branch has no errors when running spotless:check.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   2m 52s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   4m 57s |  |  the patch passed  |
   | +1 :green_heart: |  cc  |   4m 57s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   4m 57s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   1m 33s |  |  the patch passed  |
   | -0 :warning: |  rubocop  |   0m 10s | [/results-rubocop.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/10/artifact/yetus-general-check/output/results-rubocop.txt) |  The patch generated 3 new + 504 unchanged - 0 fixed = 507 total (was 504)  |
   | +1 :green_heart: |  spotbugs  |   4m 47s |  |  the patch passed  |
   | +1 :green_heart: |  hadoopcheck  |  10m 54s |  |  Patch does not cause any errors with Hadoop 3.3.6 3.4.1.  |
   | +1 :green_heart: |  hbaseprotoc  |   1m 43s |  |  the patch passed  |
   | +1 :green_heart: |  spotless  |   0m 42s |  |  patch has no errors when running spotless:check.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 35s |  |  The patch does not generate ASF License warnings.  |
   |  |   |  51m 17s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/10/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless cc buflint bufcompat hbaseprotoc rubocop |
   | uname | Linux 60c45fd91439 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 46b153346aa73a6d9c0605a931513f6e5a6d447c |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | Max. process+thread count | 85 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/10/console |
   | versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 rubocop=1.37.1 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "sidkhillon (via GitHub)" <gi...@apache.org>.
sidkhillon commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3694181766

   > Since we have a Configuration Object in ReplicationPeerConfig, what about just create a combined Configuration instance and use it when creating ReplicationSource? In this way you can directly change the sleepForRetry through the configuration.
   
   Just to confirm, you're suggesting I use the existing `configuration` map in ReplicationPeerConfig and store the value as "replication.source.sleepforretries". Then, we can override the config by doing something like:
   
   ```java
   Configuration combinedConf = new Configuration(globalConf);
   // Override any values in globalConf with the existing value in peerConfig
   peerConfig.getConfiguration().forEach(combinedConf::set);
   // set this.conf = combinedConf in the ReplicationSource
   ```
   
   Is that the approach you are looking for?


-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "taklwu (via GitHub)" <gi...@apache.org>.
taklwu commented on code in PR #7577:
URL: https://github.com/apache/hbase/pull/7577#discussion_r2653816723


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/PeerProcedureHandlerImpl.java:
##########
@@ -108,6 +109,33 @@ public void disablePeer(String peerId) throws ReplicationException, IOException
     refreshPeerState(peerId);
   }
 
+  private boolean hasReplicationConfigChange(ReplicationPeerConfig oldConfig,
+    ReplicationPeerConfig newConfig) {
+    Map<String, String> oldReplicationConfigs = oldConfig.getConfiguration();
+    Map<String, String> newReplicationConfigs = newConfig.getConfiguration();
+
+    // Check if any replication.source.* keys have changed values
+    for (Map.Entry<String, String> entry : newReplicationConfigs.entrySet()) {
+      String key = entry.getKey();
+      if (key.startsWith("replication.source.")) {

Review Comment:
   do we need to handle the following keys? if not, then your change looks good to me
   
   hbase.replication.source.fs.conf.provider
   hbase.replication.source.service
   hbase.replication.source.maxthreads



-- 
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: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-29796 Allow sleepForRetry replication config to be overridden by replication peers [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7577:
URL: https://github.com/apache/hbase/pull/7577#issuecomment-3700854761

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 28s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any anti-patterns.  |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 18s |  |  master passed  |
   | +1 :green_heart: |  compile  |   4m 58s |  |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 31s |  |  master passed  |
   | +1 :green_heart: |  spotbugs  |   4m 34s |  |  master passed  |
   | +1 :green_heart: |  spotless  |   0m 49s |  |  branch has no errors when running spotless:check.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   2m 53s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   5m  0s |  |  the patch passed  |
   | +1 :green_heart: |  cc  |   5m  0s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   5m  0s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   1m 31s |  |  the patch passed  |
   | -0 :warning: |  rubocop  |   0m 10s | [/results-rubocop.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/5/artifact/yetus-general-check/output/results-rubocop.txt) |  The patch generated 3 new + 504 unchanged - 0 fixed = 507 total (was 504)  |
   | +1 :green_heart: |  spotbugs  |   4m 50s |  |  the patch passed  |
   | +1 :green_heart: |  hadoopcheck  |  11m  9s |  |  Patch does not cause any errors with Hadoop 3.3.6 3.4.1.  |
   | +1 :green_heart: |  hbaseprotoc  |   1m 47s |  |  the patch passed  |
   | +1 :green_heart: |  spotless  |   0m 42s |  |  patch has no errors when running spotless:check.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 37s |  |  The patch does not generate ASF License warnings.  |
   |  |   |  52m 28s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/5/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7577 |
   | Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless cc buflint bufcompat hbaseprotoc rubocop |
   | uname | Linux d11cb7dbc205 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 72e9655756809634724de8123fb9f825f40460b8 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | Max. process+thread count | 85 (vs. ulimit of 30000) |
   | modules | C: hbase-protocol-shaded hbase-client hbase-server hbase-shell U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7577/5/console |
   | versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 rubocop=1.37.1 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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: issues-unsubscribe@hbase.apache.org

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