You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "umesh9794 (via GitHub)" <gi...@apache.org> on 2026/06/17 10:12:22 UTC
[PR] HDDS-15582. Further speed up TestUnhealthyContainersDerbyPerformance [ozone]
umesh9794 opened a new pull request, #10532:
URL: https://github.com/apache/ozone/pull/10532
## What changes were proposed in this pull request?
Further speed up TestUnhealthyContainersDerbyPerformance by using `BETWEEN` predicate instead of many `IN` clauses while deleting the contiguous container IDs
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15582
Please create an issue in ASF JIRA before opening a pull request, and you need to set the title of the pull
request which starts with the corresponding JIRA issue number. (e.g. HDDS-XXXX. Fix a typo in YYY.)
If you do not have an ASF Jira account yet, please follow the first-time contributor
instructions in the [Jira guideline](../CONTRIBUTING.md#jira-guideline).
(Please replace this section with the link to the Apache JIRA)
## How was this patch tested?
Ran the test locally and the elapsed time of this run is significantly reduced from ~313s to ~59s:
```
[INFO] Running org.apache.hadoop.ozone.recon.persistence.TestUnhealthyContainersDerbyPerformance
[INFO] Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 58.63 s -- in org.apache.hadoop.ozone.recon.persistence.TestUnhealthyContainersDerbyPerformance
```
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15582. Further speed up TestUnhealthyContainersDerbyPerformance [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #10532:
URL: https://github.com/apache/ozone/pull/10532#issuecomment-4729421578
Thanks @umesh9794 for the patch. Please enable the `build-branch` [workflow](https://github.com/umesh9794/ozone/actions) in your fork.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15582. Further speed up TestUnhealthyContainersDerbyPerformance [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on PR #10532:
URL: https://github.com/apache/ozone/pull/10532#issuecomment-4739528788
> @ArafatKhan2198 @devmadhuu `ContainerHealthSchemaManager.batchDeleteSCMStatesForContainers` is only called from tests, can we delete it?
Yes, we can remove it as long as our test uses some other real code logic hitting the actual product code and able to test the batch delete performance.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15601. Improve ContainerHealthSchemaManager delete performance [ozone]
Posted by "umesh9794 (via GitHub)" <gi...@apache.org>.
umesh9794 closed pull request #10532: HDDS-15601. Improve ContainerHealthSchemaManager delete performance
URL: https://github.com/apache/ozone/pull/10532
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15601. Improve ContainerHealthSchemaManager delete performance [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #10532:
URL: https://github.com/apache/ozone/pull/10532#issuecomment-4763047067
> > So I suggest splitting the task:
> > ```
> > 1. "Further speed up TestUnhealthyContainersDerbyPerformance": remove test case for `batchDeleteSCMStatesForContainers`
> >
> > 2. "Improve deleteScmStatesForContainers performance": this PR, needs further work
> > ```
>
> Yes. I agree.
Created #10551 for (1).
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15601. Improve ContainerHealthSchemaManager delete performance [ozone]
Posted by "umesh9794 (via GitHub)" <gi...@apache.org>.
umesh9794 commented on code in PR #10532:
URL: https://github.com/apache/ozone/pull/10532#discussion_r3435309580
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/persistence/ContainerHealthSchemaManager.java:
##########
@@ -205,28 +205,75 @@ public void replaceUnhealthyContainerRecordsAtomically(
private int deleteScmStatesForContainers(DSLContext dslContext,
List<Long> containerIds) {
+ if (containerIds.isEmpty()) {
+ return 0;
+ }
+
+ List<Long> sortedIds = containerIds.stream()
+ .distinct()
+ .sorted()
+ .collect(Collectors.toList());
+
int totalDeleted = 0;
+ List<Long> inClauseBatch = new ArrayList<>(MAX_IN_CLAUSE_CHUNK_SIZE);
+
+ for (int i = 0; i < sortedIds.size(); ) {
+ int rangeStart = i;
Review Comment:
@adoroszlai sure, trying to check the logic to improve the performance. 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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15601. Improve ContainerHealthSchemaManager delete performance [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on PR #10532:
URL: https://github.com/apache/ozone/pull/10532#issuecomment-4740935611
> > > `ContainerHealthSchemaManager.batchDeleteSCMStatesForContainers` is only called from tests, can we delete it?
> >
> >
> > Yes, we can remove it as long as our test uses some other real code logic hitting the actual product code and able to test the batch delete performance.
>
> Thanks @devmadhuu for checking. Yes, product code uses another method:
>
> https://github.com/apache/ozone/blob/fa1b3a35f18c14d40c32dca7bb2595c69276dd3a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/fsck/ReconReplicationManager.java#L498-L504
>
> which is also covered by the performance test:
>
> https://github.com/apache/ozone/blob/fa1b3a35f18c14d40c32dca7bb2595c69276dd3a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/persistence/TestUnhealthyContainersDerbyPerformance.java#L635
>
> So I suggest splitting the task:
>
> 1. "Further speed up TestUnhealthyContainersDerbyPerformance": remove test case for `batchDeleteSCMStatesForContainers`
>
> 2. "Improve deleteScmStatesForContainers performance": this PR, needs further work
Yes. I agree.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15601. Improve ContainerHealthSchemaManager delete performance [ozone]
Posted by "umesh9794 (via GitHub)" <gi...@apache.org>.
umesh9794 commented on code in PR #10532:
URL: https://github.com/apache/ozone/pull/10532#discussion_r3435276743
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/persistence/ContainerHealthSchemaManager.java:
##########
@@ -205,28 +205,75 @@ public void replaceUnhealthyContainerRecordsAtomically(
private int deleteScmStatesForContainers(DSLContext dslContext,
List<Long> containerIds) {
+ if (containerIds.isEmpty()) {
+ return 0;
+ }
+
+ List<Long> sortedIds = containerIds.stream()
+ .distinct()
+ .sorted()
+ .collect(Collectors.toList());
+
int totalDeleted = 0;
+ List<Long> inClauseBatch = new ArrayList<>(MAX_IN_CLAUSE_CHUNK_SIZE);
+
+ for (int i = 0; i < sortedIds.size(); ) {
+ int rangeStart = i;
Review Comment:
@devmadhuu yes current code executes more delete statements and it might slowdown in the real production envs. Let me try to optimize this for production scenarios. 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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15582. Further speed up TestUnhealthyContainersDerbyPerformance [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #10532:
URL: https://github.com/apache/ozone/pull/10532#discussion_r3434745054
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/persistence/ContainerHealthSchemaManager.java:
##########
@@ -205,28 +205,75 @@ public void replaceUnhealthyContainerRecordsAtomically(
private int deleteScmStatesForContainers(DSLContext dslContext,
List<Long> containerIds) {
+ if (containerIds.isEmpty()) {
+ return 0;
+ }
+
+ List<Long> sortedIds = containerIds.stream()
+ .distinct()
+ .sorted()
+ .collect(Collectors.toList());
+
int totalDeleted = 0;
+ List<Long> inClauseBatch = new ArrayList<>(MAX_IN_CLAUSE_CHUNK_SIZE);
+
+ for (int i = 0; i < sortedIds.size(); ) {
+ int rangeStart = i;
Review Comment:
I think we can reduce delete statement count by combining `BETWEEN` and `IN` clauses in the same query using `OR` logic. Delay submitting the statement if number of items for `IN` reaches the limit or number of `BETWEEN` ranges reaches another (lower) threshold.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15582. Further speed up TestUnhealthyContainersDerbyPerformance [ozone]
Posted by "umesh9794 (via GitHub)" <gi...@apache.org>.
umesh9794 commented on PR #10532:
URL: https://github.com/apache/ozone/pull/10532#issuecomment-4737863050
@adoroszlai I have enabled the workflow in my fork. Thanks.
Yes as I was not aware of the codebase very much, this method `ContainerHealthSchemaManager.batchDeleteSCMStatesForContainers` only gets called from the couple of test classes. Upon conformation of @ArafatKhan2198 and @devmadhuu we can clean-up this probably.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15582. Further speed up TestUnhealthyContainersDerbyPerformance [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #10532:
URL: https://github.com/apache/ozone/pull/10532#issuecomment-4740462562
> > `ContainerHealthSchemaManager.batchDeleteSCMStatesForContainers` is only called from tests, can we delete it?
>
> Yes, we can remove it as long as our test uses some other real code logic hitting the actual product code and able to test the batch delete performance.
Thanks @devmadhuu for checking. Yes, product code uses another method:
https://github.com/apache/ozone/blob/fa1b3a35f18c14d40c32dca7bb2595c69276dd3a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/fsck/ReconReplicationManager.java#L498-L504
which is also covered by the performance test:
https://github.com/apache/ozone/blob/fa1b3a35f18c14d40c32dca7bb2595c69276dd3a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/persistence/TestUnhealthyContainersDerbyPerformance.java#L635
So I suggest splitting the task:
1. "Further speed up TestUnhealthyContainersDerbyPerformance": remove test case for `batchDeleteSCMStatesForContainers`
2. "Improve deleteScmStatesForContainers performance": this PR, needs further work
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15582. Further speed up TestUnhealthyContainersDerbyPerformance [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #10532:
URL: https://github.com/apache/ozone/pull/10532#issuecomment-4730490544
@ArafatKhan2198 @devmadhuu `ContainerHealthSchemaManager.batchDeleteSCMStatesForContainers` is only called from tests, can we delete 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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15582. Further speed up TestUnhealthyContainersDerbyPerformance [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #10532:
URL: https://github.com/apache/ozone/pull/10532#discussion_r3434144018
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/persistence/ContainerHealthSchemaManager.java:
##########
@@ -205,28 +205,75 @@ public void replaceUnhealthyContainerRecordsAtomically(
private int deleteScmStatesForContainers(DSLContext dslContext,
List<Long> containerIds) {
+ if (containerIds.isEmpty()) {
+ return 0;
+ }
+
+ List<Long> sortedIds = containerIds.stream()
+ .distinct()
+ .sorted()
+ .collect(Collectors.toList());
+
int totalDeleted = 0;
+ List<Long> inClauseBatch = new ArrayList<>(MAX_IN_CLAUSE_CHUNK_SIZE);
+
+ for (int i = 0; i < sortedIds.size(); ) {
+ int rangeStart = i;
Review Comment:
This below code assumption seems incorrect that in real cluster that the unhealthy container ids all will be in continous sequence.
Real container IDs may not form one continuous sequence.
Consider this input:
`1, 2, 4, 5, 7, 8, 10, 11`
The PR sees four small continuous ranges and executes:
BETWEEN 1 AND 2
BETWEEN 4 AND 5
BETWEEN 7 AND 8
BETWEEN 10 AND 11
That means four separate DELETE statements.
The old implementation could delete all eight IDs using one statement:
`WHERE container_id IN (1, 2, 4, 5, 7, 8, 10, 11)`
With a larger realistic list containing many small pairs, the difference could become:
Old code: 50 DELETE statements
New code: 10,000 DELETE statements
Each statement must be compiled and executed by Derby. Consequently, production could become significantly slower even though this test becomes faster.
`1, 2, 3, 4, ... 200,000`
That is the best possible input for BETWEEN.
It does not test inputs such as:
1, 2, 10, 11, 20, 21, ...
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15582. Further speed up TestUnhealthyContainersDerbyPerformance [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #10532:
URL: https://github.com/apache/ozone/pull/10532#issuecomment-4730460674
> Further speed up TestUnhealthyContainersDerbyPerformance by using `BETWEEN` predicate instead of many `IN` clauses while deleting the contiguous container IDs
While the speed improvement in `TestUnhealthyContainersDerbyPerformance` is nice (173 -> 51 seconds locally), the test no longer seems to simulate real workload, where a continuous range of 200K containers is unlikely.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org