You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "mnpoonia (via GitHub)" <gi...@apache.org> on 2025/12/24 15:38:23 UTC
[PR] HBASE-29706: Make CoprocessorDescriptorImpl.equals() and hashCode() r… [hbase]
mnpoonia opened a new pull request, #7575:
URL: https://github.com/apache/hbase/pull/7575
…obust
--
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-29706: Make CoprocessorDescriptorImpl.equals() and hashCode() r… [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7575:
URL: https://github.com/apache/hbase/pull/7575#issuecomment-3690438304
:confetti_ball: **+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 | 4m 38s | | master passed |
| +1 :green_heart: | compile | 1m 47s | | master passed |
| +1 :green_heart: | javadoc | 1m 4s | | master passed |
| +1 :green_heart: | shadedjars | 7m 8s | | 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 13s | | the patch passed |
| +1 :green_heart: | compile | 1m 43s | | the patch passed |
| +1 :green_heart: | javac | 1m 43s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 57s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 58s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 5s | | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 219m 1s | | hbase-server in the patch passed. |
| | | 256m 24s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/1/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7575 |
| JIRA Issue | HBASE-29706 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 60437fb53370 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 / cd7471776b5686548ae2091f4b6abbedaa0d6953 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/1/testReport/ |
| Max. process+thread count | 4500 (vs. ulimit of 30000) |
| modules | C: hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/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-29706: Make CoprocessorDescriptorImpl.equals() and hashCode() r… [hbase]
Posted by "Umeshkumar9414 (via GitHub)" <gi...@apache.org>.
Umeshkumar9414 commented on code in PR #7575:
URL: https://github.com/apache/hbase/pull/7575#discussion_r2647526905
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyTableProcedure.java:
##########
@@ -698,4 +699,72 @@ public void testModifyWillNotReopenRegions() throws IOException {
assertTrue(e.getMessage().contains("Can not modify REGION_REPLICATION"));
}
}
+
+ @Test
+ public void testModifyTableWithCoprocessorAndColumnFamilyPropertyChange() throws IOException {
+ // HBASE-29706 - This test validates the fix for the bug where modifying only column family
+ // properties
+ // (like COMPRESSION) with REOPEN_REGIONS=false would incorrectly throw an error when
+ // coprocessors are present. The bug was caused by comparing collection hash codes
+ // instead of actual descriptor content, which failed when HashMap iteration order varied.
+
+ final boolean reopenRegions = false;
+ final TableName tableName = TableName.valueOf(name.getMethodName());
+ final ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
+
+ MasterProcedureTestingUtility.createTable(procExec, tableName, null, "cf");
+
+ // Step 1: Add a coprocessor to the table
+ TableDescriptor htd = UTIL.getAdmin().getDescriptor(tableName);
+ TableDescriptor descriptorWithCoprocessor =
+ TableDescriptorBuilder.newBuilder(htd).setCoprocessor(CoprocessorDescriptorBuilder
+ .newBuilder(SimpleRegionObserver.class.getName()).setPriority(100).build()).build();
Review Comment:
nit: maybe we can add one more coprocessor, to also test that equals is working with multiple coprocessor
--
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-29706: Make CoprocessorDescriptorImpl.equals() and hashCode() r… [hbase]
Posted by "mnpoonia (via GitHub)" <gi...@apache.org>.
mnpoonia commented on PR #7575:
URL: https://github.com/apache/hbase/pull/7575#issuecomment-3693037762
@apurtell @virajjasani please review when you get some free cycle.
--
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-29706: Alter table with reopen false should pass if coprocessors havenot changed [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7575:
URL: https://github.com/apache/hbase/pull/7575#issuecomment-3698653813
:confetti_ball: **+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 34s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 34s | | master passed |
| +1 :green_heart: | compile | 1m 22s | | master passed |
| +1 :green_heart: | javadoc | 0m 45s | | 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 7s | | the patch passed |
| +1 :green_heart: | compile | 1m 22s | | the patch passed |
| +1 :green_heart: | javac | 1m 22s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 45s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 14s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 32s | | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 212m 36s | | hbase-server in the patch passed. |
| | | 243m 55s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/5/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7575 |
| JIRA Issue | HBASE-29706 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 54ad26445290 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 / 300fb24682a86c26b4b073b06c45b572e9ca7eda |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/5/testReport/ |
| Max. process+thread count | 4557 (vs. ulimit of 30000) |
| modules | C: hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/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-29706: Alter table with reopen false should pass if coprocessors havenot changed [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7575:
URL: https://github.com/apache/hbase/pull/7575#issuecomment-3701814212
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 31s | | 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. |
| +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 33s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 15s | | master passed |
| +1 :green_heart: | compile | 5m 21s | | master passed |
| +1 :green_heart: | checkstyle | 1m 21s | | master passed |
| +1 :green_heart: | spotbugs | 3m 17s | | master passed |
| +1 :green_heart: | spotless | 1m 22s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 19s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 17s | | the patch passed |
| +1 :green_heart: | compile | 4m 40s | | the patch passed |
| +1 :green_heart: | javac | 4m 40s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 1m 24s | | the patch passed |
| +1 :green_heart: | spotbugs | 2m 37s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 11m 52s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.1. |
| +1 :green_heart: | spotless | 0m 47s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 20s | | The patch does not generate ASF License warnings. |
| | | 50m 56s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/6/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7575 |
| JIRA Issue | HBASE-29706 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux 8b251f9da0e0 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 / c5aca560bb21c96ba6eb0d516013166c9becebbd |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 83 (vs. ulimit of 30000) |
| modules | C: hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/6/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| 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-29706: Alter table with reopen false should pass if coprocessors havenot changed [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7575:
URL: https://github.com/apache/hbase/pull/7575#issuecomment-3696568102
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 1m 20s | | 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 13s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 2m 48s | | master passed |
| +1 :green_heart: | compile | 1m 2s | | master passed |
| +1 :green_heart: | javadoc | 0m 35s | | master passed |
| +1 :green_heart: | shadedjars | 4m 34s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 15s | | the patch passed |
| +1 :green_heart: | compile | 1m 2s | | the patch passed |
| +1 :green_heart: | javac | 1m 2s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 34s | | the patch passed |
| +1 :green_heart: | shadedjars | 4m 30s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 12s | | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 209m 54s | | hbase-server in the patch passed. |
| | | 234m 6s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.48 ServerAPI=1.48 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/4/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7575 |
| JIRA Issue | HBASE-29706 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 9a93d7abde05 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 / d66e3acddd5ecff29497bb0a0ee2ff8e9a9ad74e |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/4/testReport/ |
| Max. process+thread count | 5612 (vs. ulimit of 30000) |
| modules | C: hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/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-29706: Make CoprocessorDescriptorImpl.equals() and hashCode() r… [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7575:
URL: https://github.com/apache/hbase/pull/7575#issuecomment-3692668384
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 44s | | 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 42s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 0s | | master passed |
| +1 :green_heart: | compile | 1m 47s | | master passed |
| +1 :green_heart: | javadoc | 1m 0s | | master passed |
| +1 :green_heart: | shadedjars | 7m 2s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 17s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 7s | | the patch passed |
| +1 :green_heart: | compile | 1m 42s | | the patch passed |
| +1 :green_heart: | javac | 1m 42s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 59s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 53s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 4s | | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 225m 10s | | hbase-server in the patch passed. |
| | | 263m 40s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/3/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7575 |
| JIRA Issue | HBASE-29706 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 02bda1973165 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 / 3a1ebef1fc61efa7a117d5ea3743515ff0c605ba |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/3/testReport/ |
| Max. process+thread count | 4068 (vs. ulimit of 30000) |
| modules | C: hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/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-29706: Alter table with reopen false should pass if coprocessors havenot changed [hbase]
Posted by "mnpoonia (via GitHub)" <gi...@apache.org>.
mnpoonia commented on code in PR #7575:
URL: https://github.com/apache/hbase/pull/7575#discussion_r2651069656
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptorBuilder.java:
##########
@@ -111,5 +114,34 @@ public String toString() {
return "class:" + className + ", jarPath:" + jarPath + ", priority:" + priority
+ ", properties:" + properties;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof CoprocessorDescriptor)) {
+ return false;
+ }
+ CoprocessorDescriptor other = (CoprocessorDescriptor) obj;
+ if (
+ priority != other.getPriority() || !Objects.equals(className, other.getClassName())
+ || !Objects.equals(getJarPath(), other.getJarPath())
+ ) {
+ return false;
+ }
+ return Objects.equals(getProperties(), other.getProperties());
+ }
+
+ @Override
+ public int hashCode() {
+ int result = Objects.hash(className, jarPath, priority);
+ List<Map.Entry<String, String>> entries = new ArrayList<>(properties.entrySet());
Review Comment:
Let me know if you want it within same jira.
--
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-29706: Alter table with reopen false should pass if coprocessors havenot changed [hbase]
Posted by "mnpoonia (via GitHub)" <gi...@apache.org>.
mnpoonia commented on code in PR #7575:
URL: https://github.com/apache/hbase/pull/7575#discussion_r2655010154
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptor.java:
##########
@@ -40,5 +40,5 @@ public interface CoprocessorDescriptor {
int getPriority();
/** Returns Arbitrary key-value parameter pairs passed into the coprocessor. */
- Map<String, String> getProperties();
+ NavigableMap<String, String> getProperties();
Review Comment:
Done.
--
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-29706: Make CoprocessorDescriptorImpl.equals() and hashCode() r… [hbase]
Posted by "mnpoonia (via GitHub)" <gi...@apache.org>.
mnpoonia commented on code in PR #7575:
URL: https://github.com/apache/hbase/pull/7575#discussion_r2650579370
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptorBuilder.java:
##########
@@ -111,5 +111,28 @@ public String toString() {
return "class:" + className + ", jarPath:" + jarPath + ", priority:" + priority
+ ", properties:" + properties;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (obj == null || !(obj instanceof CoprocessorDescriptor)) {
+ return false;
+ }
+ CoprocessorDescriptor other = (CoprocessorDescriptor) obj;
+ if (
+ priority != other.getPriority() || !Objects.equals(className, other.getClassName())
+ || !Objects.equals(getJarPath(), other.getJarPath())
+ ) {
+ return false;
+ }
+ return new TreeMap<>(getProperties()).equals(new TreeMap<>(other.getProperties()));
Review Comment:
Fixed and improved it.
--
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-29706: Alter table with reopen false should pass if coprocessors havenot changed [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7575:
URL: https://github.com/apache/hbase/pull/7575#issuecomment-3698345924
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 39s | | 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. |
| +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 38s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 13s | | master passed |
| +1 :green_heart: | compile | 5m 11s | | master passed |
| +1 :green_heart: | checkstyle | 1m 36s | | master passed |
| +1 :green_heart: | spotbugs | 3m 4s | | master passed |
| +1 :green_heart: | spotless | 0m 57s | | 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 | 3m 41s | | the patch passed |
| +1 :green_heart: | compile | 5m 5s | | the patch passed |
| +1 :green_heart: | javac | 5m 5s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 1m 36s | | the patch passed |
| +1 :green_heart: | spotbugs | 3m 12s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 14m 4s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.1. |
| +1 :green_heart: | spotless | 0m 52s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 21s | | The patch does not generate ASF License warnings. |
| | | 54m 1s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7575 |
| JIRA Issue | HBASE-29706 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux 7531838daed9 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 / 300fb24682a86c26b4b073b06c45b572e9ca7eda |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 85 (vs. ulimit of 30000) |
| modules | C: hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/5/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| 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-29706: Alter table with reopen false should pass if coprocessors havenot changed [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on code in PR #7575:
URL: https://github.com/apache/hbase/pull/7575#discussion_r2654674095
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptor.java:
##########
@@ -40,5 +40,5 @@ public interface CoprocessorDescriptor {
int getPriority();
/** Returns Arbitrary key-value parameter pairs passed into the coprocessor. */
- Map<String, String> getProperties();
+ NavigableMap<String, String> getProperties();
Review Comment:
This interface is IA.Public so let's keep it as is.
--
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-29706: Make CoprocessorDescriptorImpl.equals() and hashCode() r… [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7575:
URL: https://github.com/apache/hbase/pull/7575#issuecomment-3690479184
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 34s | | 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. |
| +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 17s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 26s | | master passed |
| +1 :green_heart: | compile | 4m 18s | | master passed |
| +1 :green_heart: | checkstyle | 1m 23s | | master passed |
| +1 :green_heart: | spotbugs | 2m 20s | | master passed |
| +1 :green_heart: | spotless | 0m 50s | | 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 18s | | the patch passed |
| +1 :green_heart: | javac | 4m 18s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 1m 20s | | the patch passed |
| +1 :green_heart: | spotbugs | 2m 33s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 11m 40s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.1. |
| +1 :green_heart: | spotless | 0m 46s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 20s | | The patch does not generate ASF License warnings. |
| | | 45m 8s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7575 |
| JIRA Issue | HBASE-29706 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux a14ca05c96e5 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 / 4272f2ce6aaf014a4f1b06b8fee2c05650db1cce |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 84 (vs. ulimit of 30000) |
| modules | C: hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/2/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| 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-29706: Make CoprocessorDescriptorImpl.equals() and hashCode() r… [hbase]
Posted by "mnpoonia (via GitHub)" <gi...@apache.org>.
mnpoonia commented on code in PR #7575:
URL: https://github.com/apache/hbase/pull/7575#discussion_r2647609291
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyTableProcedure.java:
##########
@@ -698,4 +699,72 @@ public void testModifyWillNotReopenRegions() throws IOException {
assertTrue(e.getMessage().contains("Can not modify REGION_REPLICATION"));
}
}
+
+ @Test
+ public void testModifyTableWithCoprocessorAndColumnFamilyPropertyChange() throws IOException {
+ // HBASE-29706 - This test validates the fix for the bug where modifying only column family
+ // properties
+ // (like COMPRESSION) with REOPEN_REGIONS=false would incorrectly throw an error when
+ // coprocessors are present. The bug was caused by comparing collection hash codes
+ // instead of actual descriptor content, which failed when HashMap iteration order varied.
+
+ final boolean reopenRegions = false;
+ final TableName tableName = TableName.valueOf(name.getMethodName());
+ final ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
+
+ MasterProcedureTestingUtility.createTable(procExec, tableName, null, "cf");
+
+ // Step 1: Add a coprocessor to the table
+ TableDescriptor htd = UTIL.getAdmin().getDescriptor(tableName);
+ TableDescriptor descriptorWithCoprocessor =
+ TableDescriptorBuilder.newBuilder(htd).setCoprocessor(CoprocessorDescriptorBuilder
+ .newBuilder(SimpleRegionObserver.class.getName()).setPriority(100).build()).build();
Review Comment:
Thanks @Umeshkumar9414 . Added second coproc to see if that changes anything.
--
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-29706: Alter table with reopen false should pass if coprocessors havenot changed [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on code in PR #7575:
URL: https://github.com/apache/hbase/pull/7575#discussion_r2650675355
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptorBuilder.java:
##########
@@ -111,5 +114,34 @@ public String toString() {
return "class:" + className + ", jarPath:" + jarPath + ", priority:" + priority
+ ", properties:" + properties;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof CoprocessorDescriptor)) {
+ return false;
+ }
+ CoprocessorDescriptor other = (CoprocessorDescriptor) obj;
+ if (
+ priority != other.getPriority() || !Objects.equals(className, other.getClassName())
+ || !Objects.equals(getJarPath(), other.getJarPath())
+ ) {
+ return false;
+ }
+ return Objects.equals(getProperties(), other.getProperties());
+ }
+
+ @Override
+ public int hashCode() {
+ int result = Objects.hash(className, jarPath, priority);
+ List<Map.Entry<String, String>> entries = new ArrayList<>(properties.entrySet());
Review Comment:
Ah, checked the code, properties is already a TreeMap... So we do not need to sort here, just iterate over it is enough...
To make it safe, we can change the declaration of properties from Map to SortedMap or NavigableMap.
--
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-29706: Make CoprocessorDescriptorImpl.equals() and hashCode() r… [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on code in PR #7575:
URL: https://github.com/apache/hbase/pull/7575#discussion_r2649109173
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptorBuilder.java:
##########
@@ -111,5 +111,28 @@ public String toString() {
return "class:" + className + ", jarPath:" + jarPath + ", priority:" + priority
+ ", properties:" + properties;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (obj == null || !(obj instanceof CoprocessorDescriptor)) {
+ return false;
+ }
+ CoprocessorDescriptor other = (CoprocessorDescriptor) obj;
+ if (
+ priority != other.getPriority() || !Objects.equals(className, other.getClassName())
+ || !Objects.equals(getJarPath(), other.getJarPath())
+ ) {
+ return false;
+ }
+ return new TreeMap<>(getProperties()).equals(new TreeMap<>(other.getProperties()));
Review Comment:
I think we could use equals directly here?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java:
##########
@@ -133,8 +133,8 @@ protected void preflightChecks(MasterProcedureEnv env, Boolean enabled) throws H
"Cannot add or remove column families when this modification " + "won't reopen regions.");
}
if (
- this.unmodifiedTableDescriptor.getCoprocessorDescriptors().hashCode()
- != this.modifiedTableDescriptor.getCoprocessorDescriptors().hashCode()
+ !new HashSet<>(this.unmodifiedTableDescriptor.getCoprocessorDescriptors())
Review Comment:
This is not on the critial read/write path so maybe this is acceptable, but a better way, is to first check size, and then, create a HashSet for one of the descriptors, and then iterator over the other one, and check whethere the HashSet contains it.
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptorBuilder.java:
##########
@@ -111,5 +111,28 @@ public String toString() {
return "class:" + className + ", jarPath:" + jarPath + ", priority:" + priority
+ ", properties:" + properties;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (obj == null || !(obj instanceof CoprocessorDescriptor)) {
+ return false;
+ }
+ CoprocessorDescriptor other = (CoprocessorDescriptor) obj;
+ if (
+ priority != other.getPriority() || !Objects.equals(className, other.getClassName())
+ || !Objects.equals(getJarPath(), other.getJarPath())
+ ) {
+ return false;
+ }
+ return new TreeMap<>(getProperties()).equals(new TreeMap<>(other.getProperties()));
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(className, jarPath, priority, new TreeMap<>(properties));
Review Comment:
Better use Objects.hash to calculate hashCode for fields other than properties, and then use stream.sorted to iterator over properties and calculate hashCode on the fly.
--
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-29706: Alter table with reopen false should pass if coprocessors havenot changed [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7575:
URL: https://github.com/apache/hbase/pull/7575#issuecomment-3702127268
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 32s | | 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 33s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 29s | | master passed |
| +1 :green_heart: | compile | 1m 22s | | master passed |
| +1 :green_heart: | javadoc | 0m 44s | | master passed |
| +1 :green_heart: | shadedjars | 6m 14s | | 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 8s | | the patch passed |
| +1 :green_heart: | compile | 1m 21s | | the patch passed |
| +1 :green_heart: | javac | 1m 21s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 44s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 14s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 34s | | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 213m 26s | | hbase-server in the patch passed. |
| | | 244m 49s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/6/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7575 |
| JIRA Issue | HBASE-29706 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 5d2283a7770c 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 / c5aca560bb21c96ba6eb0d516013166c9becebbd |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/6/testReport/ |
| Max. process+thread count | 4579 (vs. ulimit of 30000) |
| modules | C: hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/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-29706: Alter table with reopen false should pass if coprocessors havenot changed [hbase]
Posted by "mnpoonia (via GitHub)" <gi...@apache.org>.
mnpoonia commented on code in PR #7575:
URL: https://github.com/apache/hbase/pull/7575#discussion_r2651068361
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptorBuilder.java:
##########
@@ -111,5 +114,34 @@ public String toString() {
return "class:" + className + ", jarPath:" + jarPath + ", priority:" + priority
+ ", properties:" + properties;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof CoprocessorDescriptor)) {
+ return false;
+ }
+ CoprocessorDescriptor other = (CoprocessorDescriptor) obj;
+ if (
+ priority != other.getPriority() || !Objects.equals(className, other.getClassName())
+ || !Objects.equals(getJarPath(), other.getJarPath())
+ ) {
+ return false;
+ }
+ return Objects.equals(getProperties(), other.getProperties());
+ }
+
+ @Override
+ public int hashCode() {
+ int result = Objects.hash(className, jarPath, priority);
+ List<Map.Entry<String, String>> entries = new ArrayList<>(properties.entrySet());
Review Comment:
Consdering this change is not in hot path should we still change the treemap to map(Navigable or sortable)?
This might become a cleanup exercise where treemap specific functionality is used for elsewhere. I mean i am happy to change it but doesn't make sense to do it in this jira atleast.
--
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-29706: Alter table with reopen false should pass if coprocessors havenot changed [hbase]
Posted by "mnpoonia (via GitHub)" <gi...@apache.org>.
mnpoonia commented on code in PR #7575:
URL: https://github.com/apache/hbase/pull/7575#discussion_r2653256805
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptorBuilder.java:
##########
@@ -111,5 +114,34 @@ public String toString() {
return "class:" + className + ", jarPath:" + jarPath + ", priority:" + priority
+ ", properties:" + properties;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof CoprocessorDescriptor)) {
+ return false;
+ }
+ CoprocessorDescriptor other = (CoprocessorDescriptor) obj;
+ if (
+ priority != other.getPriority() || !Objects.equals(className, other.getClassName())
+ || !Objects.equals(getJarPath(), other.getJarPath())
+ ) {
+ return false;
+ }
+ return Objects.equals(getProperties(), other.getProperties());
+ }
+
+ @Override
+ public int hashCode() {
+ int result = Objects.hash(className, jarPath, priority);
+ List<Map.Entry<String, String>> entries = new ArrayList<>(properties.entrySet());
Review Comment:
@Apache9 if you think we should still calculate hash for properties on the fly and use object hash for others let me know. I think current logic of just using objects.hash should suffice here after our change in equals logic and change of properties to navigablemap
--
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-29706 Modify table with lazy mode should pass if coprocessors have not changed [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 merged PR #7575:
URL: https://github.com/apache/hbase/pull/7575
--
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-29706: Alter table with reopen false should pass if coprocessors havenot changed [hbase]
Posted by "mnpoonia (via GitHub)" <gi...@apache.org>.
mnpoonia commented on code in PR #7575:
URL: https://github.com/apache/hbase/pull/7575#discussion_r2652149934
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptorBuilder.java:
##########
@@ -111,5 +114,34 @@ public String toString() {
return "class:" + className + ", jarPath:" + jarPath + ", priority:" + priority
+ ", properties:" + properties;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof CoprocessorDescriptor)) {
+ return false;
+ }
+ CoprocessorDescriptor other = (CoprocessorDescriptor) obj;
+ if (
+ priority != other.getPriority() || !Objects.equals(className, other.getClassName())
+ || !Objects.equals(getJarPath(), other.getJarPath())
+ ) {
+ return false;
+ }
+ return Objects.equals(getProperties(), other.getProperties());
+ }
+
+ @Override
+ public int hashCode() {
+ int result = Objects.hash(className, jarPath, priority);
+ List<Map.Entry<String, String>> entries = new ArrayList<>(properties.entrySet());
Review Comment:
Thanks @Apache9 . I think after we move to NavigableMap we can change the whole hashCode() method to a simple Objects.hash(className, jarPath, priority, properties);
This will be exactly equivalent to current logic.
--
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-29706: Make CoprocessorDescriptorImpl.equals() and hashCode() r… [hbase]
Posted by "Umeshkumar9414 (via GitHub)" <gi...@apache.org>.
Umeshkumar9414 commented on PR #7575:
URL: https://github.com/apache/hbase/pull/7575#issuecomment-3690366153
Need to run `mvn spotless:apply`
--
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-29706: Make CoprocessorDescriptorImpl.equals() and hashCode() r… [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7575:
URL: https://github.com/apache/hbase/pull/7575#issuecomment-3690223168
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 42s | | 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. |
| +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 | 4m 40s | | master passed |
| +1 :green_heart: | compile | 5m 22s | | master passed |
| +1 :green_heart: | checkstyle | 1m 29s | | master passed |
| +1 :green_heart: | spotbugs | 2m 50s | | master passed |
| +1 :green_heart: | spotless | 1m 10s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 59s | | the patch passed |
| +1 :green_heart: | compile | 5m 3s | | the patch passed |
| +1 :green_heart: | javac | 5m 3s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 1m 7s | [/results-checkstyle-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/1/artifact/yetus-general-check/output/results-checkstyle-hbase-server.txt) | hbase-server: The patch generated 1 new + 1 unchanged - 0 fixed = 2 total (was 1) |
| +1 :green_heart: | spotbugs | 3m 32s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 17m 58s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.1. |
| -1 :x: | spotless | 0m 47s | | patch has 63 errors when running spotless:check, run spotless:apply to fix. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 22s | | The patch does not generate ASF License warnings. |
| | | 59m 39s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7575 |
| JIRA Issue | HBASE-29706 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux 68895902d2c8 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 / cd7471776b5686548ae2091f4b6abbedaa0d6953 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| spotless | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/1/artifact/yetus-general-check/output/patch-spotless.txt |
| Max. process+thread count | 85 (vs. ulimit of 30000) |
| modules | C: hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/1/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| 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-29706: Make CoprocessorDescriptorImpl.equals() and hashCode() r… [hbase]
Posted by "mnpoonia (via GitHub)" <gi...@apache.org>.
mnpoonia commented on code in PR #7575:
URL: https://github.com/apache/hbase/pull/7575#discussion_r2650583810
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptorBuilder.java:
##########
@@ -111,5 +111,28 @@ public String toString() {
return "class:" + className + ", jarPath:" + jarPath + ", priority:" + priority
+ ", properties:" + properties;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (obj == null || !(obj instanceof CoprocessorDescriptor)) {
+ return false;
+ }
+ CoprocessorDescriptor other = (CoprocessorDescriptor) obj;
+ if (
+ priority != other.getPriority() || !Objects.equals(className, other.getClassName())
+ || !Objects.equals(getJarPath(), other.getJarPath())
+ ) {
+ return false;
+ }
+ return new TreeMap<>(getProperties()).equals(new TreeMap<>(other.getProperties()));
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(className, jarPath, priority, new TreeMap<>(properties));
Review Comment:
Done. Almost what you suggested.
--
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-29706: Make CoprocessorDescriptorImpl.equals() and hashCode() r… [hbase]
Posted by "mnpoonia (via GitHub)" <gi...@apache.org>.
mnpoonia commented on code in PR #7575:
URL: https://github.com/apache/hbase/pull/7575#discussion_r2650583097
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java:
##########
@@ -133,8 +133,8 @@ protected void preflightChecks(MasterProcedureEnv env, Boolean enabled) throws H
"Cannot add or remove column families when this modification " + "won't reopen regions.");
}
if (
- this.unmodifiedTableDescriptor.getCoprocessorDescriptors().hashCode()
- != this.modifiedTableDescriptor.getCoprocessorDescriptors().hashCode()
+ !new HashSet<>(this.unmodifiedTableDescriptor.getCoprocessorDescriptors())
Review Comment:
Done.
--
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-29706: Alter table with reopen false should pass if coprocessors havenot changed [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7575:
URL: https://github.com/apache/hbase/pull/7575#issuecomment-3696085460
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 1m 20s | | 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. |
| +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 13s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 2m 45s | | master passed |
| +1 :green_heart: | compile | 3m 11s | | master passed |
| +1 :green_heart: | checkstyle | 0m 59s | | master passed |
| +1 :green_heart: | spotbugs | 1m 46s | | master passed |
| +1 :green_heart: | spotless | 0m 40s | | 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 16s | | the patch passed |
| +1 :green_heart: | compile | 3m 13s | | the patch passed |
| +1 :green_heart: | javac | 3m 13s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 59s | | the patch passed |
| +1 :green_heart: | spotbugs | 1m 55s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 8m 30s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.1. |
| +1 :green_heart: | spotless | 0m 34s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 14s | | The patch does not generate ASF License warnings. |
| | | 34m 13s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.48 ServerAPI=1.48 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7575 |
| JIRA Issue | HBASE-29706 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux a82d5bea3329 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 / d66e3acddd5ecff29497bb0a0ee2ff8e9a9ad74e |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 85 (vs. ulimit of 30000) |
| modules | C: hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/4/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| 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-29706: Alter table with reopen false should pass if coprocessors havenot changed [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on code in PR #7575:
URL: https://github.com/apache/hbase/pull/7575#discussion_r2651120794
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptorBuilder.java:
##########
@@ -111,5 +114,34 @@ public String toString() {
return "class:" + className + ", jarPath:" + jarPath + ", priority:" + priority
+ ", properties:" + properties;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof CoprocessorDescriptor)) {
+ return false;
+ }
+ CoprocessorDescriptor other = (CoprocessorDescriptor) obj;
+ if (
+ priority != other.getPriority() || !Objects.equals(className, other.getClassName())
+ || !Objects.equals(getJarPath(), other.getJarPath())
+ ) {
+ return false;
+ }
+ return Objects.equals(getProperties(), other.getProperties());
+ }
+
+ @Override
+ public int hashCode() {
+ int result = Objects.hash(className, jarPath, priority);
+ List<Map.Entry<String, String>> entries = new ArrayList<>(properties.entrySet());
Review Comment:
I think it is related. We want to make sure the hash code calculation is stable, so here we want to sort the entries before calculating. Since we already use TreeMap in CoprocessorDescriptorBuilder, we can make sure that the Map in CoprocessorDescriptor is a TreeMap, which is sorted, so we do not need to sort it again.
To prevent later developers break the assumption, i.e, changing the TreeMap to HashMap in CoprocessorDescriptorBuilder without any compilation errors, we'd better change the declaration in CoprocessorDescriptor to SortedMap or NavigableMap, so the constructor will not accept a HashMap any more. And we could also add some comments to explain that we must use SortedMap here as we need it for a stable hash code calculation.
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-29706: Make CoprocessorDescriptorImpl.equals() and hashCode() r… [hbase]
Posted by "mnpoonia (via GitHub)" <gi...@apache.org>.
mnpoonia commented on code in PR #7575:
URL: https://github.com/apache/hbase/pull/7575#discussion_r2647609291
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyTableProcedure.java:
##########
@@ -698,4 +699,72 @@ public void testModifyWillNotReopenRegions() throws IOException {
assertTrue(e.getMessage().contains("Can not modify REGION_REPLICATION"));
}
}
+
+ @Test
+ public void testModifyTableWithCoprocessorAndColumnFamilyPropertyChange() throws IOException {
+ // HBASE-29706 - This test validates the fix for the bug where modifying only column family
+ // properties
+ // (like COMPRESSION) with REOPEN_REGIONS=false would incorrectly throw an error when
+ // coprocessors are present. The bug was caused by comparing collection hash codes
+ // instead of actual descriptor content, which failed when HashMap iteration order varied.
+
+ final boolean reopenRegions = false;
+ final TableName tableName = TableName.valueOf(name.getMethodName());
+ final ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
+
+ MasterProcedureTestingUtility.createTable(procExec, tableName, null, "cf");
+
+ // Step 1: Add a coprocessor to the table
+ TableDescriptor htd = UTIL.getAdmin().getDescriptor(tableName);
+ TableDescriptor descriptorWithCoprocessor =
+ TableDescriptorBuilder.newBuilder(htd).setCoprocessor(CoprocessorDescriptorBuilder
+ .newBuilder(SimpleRegionObserver.class.getName()).setPriority(100).build()).build();
Review Comment:
Thanks @Umeshkumar9414 . Added second coproc to make test robust.
--
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-29706: Make CoprocessorDescriptorImpl.equals() and hashCode() r… [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7575:
URL: https://github.com/apache/hbase/pull/7575#issuecomment-3692316119
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 47s | | 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. |
| +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 38s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 58s | | master passed |
| +1 :green_heart: | compile | 5m 18s | | master passed |
| +1 :green_heart: | checkstyle | 1m 30s | | master passed |
| +1 :green_heart: | spotbugs | 3m 11s | | master passed |
| +1 :green_heart: | spotless | 1m 11s | | 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 | 3m 51s | | the patch passed |
| +1 :green_heart: | compile | 5m 1s | | the patch passed |
| +1 :green_heart: | javac | 5m 1s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 1m 25s | | the patch passed |
| +1 :green_heart: | spotbugs | 3m 24s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 17m 42s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.1. |
| +1 :green_heart: | spotless | 0m 55s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 22s | | The patch does not generate ASF License warnings. |
| | | 60m 22s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7575 |
| JIRA Issue | HBASE-29706 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux 87a1764ea28a 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 / 3a1ebef1fc61efa7a117d5ea3743515ff0c605ba |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 84 (vs. ulimit of 30000) |
| modules | C: hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/3/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| 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-29706: Make CoprocessorDescriptorImpl.equals() and hashCode() r… [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7575:
URL: https://github.com/apache/hbase/pull/7575#issuecomment-3690660103
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 35s | | 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 17s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 7s | | master passed |
| +1 :green_heart: | compile | 1m 42s | | master passed |
| +1 :green_heart: | javadoc | 1m 2s | | master passed |
| +1 :green_heart: | shadedjars | 7m 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 55s | | the patch passed |
| +1 :green_heart: | compile | 1m 45s | | the patch passed |
| +1 :green_heart: | javac | 1m 45s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 56s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 57s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 47s | | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 246m 29s | | hbase-server in the patch passed. |
| | | 282m 49s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/2/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7575 |
| JIRA Issue | HBASE-29706 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 12a1c5c8afb2 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 / 4272f2ce6aaf014a4f1b06b8fee2c05650db1cce |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/2/testReport/ |
| Max. process+thread count | 4003 (vs. ulimit of 30000) |
| modules | C: hbase-client hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7575/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-29706: Alter table with reopen false should pass if coprocessors havenot changed [hbase]
Posted by "mnpoonia (via GitHub)" <gi...@apache.org>.
mnpoonia commented on code in PR #7575:
URL: https://github.com/apache/hbase/pull/7575#discussion_r2652185464
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptorBuilder.java:
##########
@@ -111,5 +114,34 @@ public String toString() {
return "class:" + className + ", jarPath:" + jarPath + ", priority:" + priority
+ ", properties:" + properties;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof CoprocessorDescriptor)) {
+ return false;
+ }
+ CoprocessorDescriptor other = (CoprocessorDescriptor) obj;
+ if (
+ priority != other.getPriority() || !Objects.equals(className, other.getClassName())
+ || !Objects.equals(getJarPath(), other.getJarPath())
+ ) {
+ return false;
+ }
+ return Objects.equals(getProperties(), other.getProperties());
+ }
+
+ @Override
+ public int hashCode() {
+ int result = Objects.hash(className, jarPath, priority);
+ List<Map.Entry<String, String>> entries = new ArrayList<>(properties.entrySet());
Review Comment:
Updated the PR. Have updated the Map to NavigableMap in interface as well. Let me know what you think about the same.
--
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-29706: Alter table with reopen false should pass if coprocessors havenot changed [hbase]
Posted by "mnpoonia (via GitHub)" <gi...@apache.org>.
mnpoonia commented on code in PR #7575:
URL: https://github.com/apache/hbase/pull/7575#discussion_r2651068361
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptorBuilder.java:
##########
@@ -111,5 +114,34 @@ public String toString() {
return "class:" + className + ", jarPath:" + jarPath + ", priority:" + priority
+ ", properties:" + properties;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof CoprocessorDescriptor)) {
+ return false;
+ }
+ CoprocessorDescriptor other = (CoprocessorDescriptor) obj;
+ if (
+ priority != other.getPriority() || !Objects.equals(className, other.getClassName())
+ || !Objects.equals(getJarPath(), other.getJarPath())
+ ) {
+ return false;
+ }
+ return Objects.equals(getProperties(), other.getProperties());
+ }
+
+ @Override
+ public int hashCode() {
+ int result = Objects.hash(className, jarPath, priority);
+ List<Map.Entry<String, String>> entries = new ArrayList<>(properties.entrySet());
Review Comment:
Consdering this change is not in hot path should we still change the treemap to map(Navigable or sortable)?
This might become a cleanup exercise where treemap specific functionality is used elsewhere. I mean i am happy to change it but doesn't make sense to do it in this jira atleast.
--
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