You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "rich7420 (via GitHub)" <gi...@apache.org> on 2025/10/18 10:29:29 UTC
[PR] HDDS-13809. Close OmMetadataManagerImpl.createCheckpointMetadataManager properly [ozone]
rich7420 opened a new pull request, #9173:
URL: https://github.com/apache/ozone/pull/9173
## What changes were proposed in this pull request?
This PR improves resource management by ensuring all OmMetadataManagerImpl instances created via createCheckpointMetadataManager() are properly closed using try-with-resources statements, following Java best practices and fixed CI test failure caused by behavior change.
## What is the link to the Apache JIRA
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.)
[HDDS-13809](https://issues.apache.org/jira/browse/HDDS-13809)
## How was this patch tested?
(Please explain how this patch was tested. Ex: unit tests, manual tests, workflow run on the fork git repo.)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: 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-13809. Close OmMetadataManagerImpl.createCheckpointMetadataManager properly [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9173:
URL: https://github.com/apache/ozone/pull/9173#discussion_r2450542167
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/QuotaRepairTask.java:
##########
@@ -102,32 +102,33 @@ public static String getStatus() {
private boolean repairTask(List<String> buckets) {
LOG.info("Starting quota repair task {}", REPAIR_STATUS);
- OMMetadataManager activeMetaManager = null;
try {
// thread pool with 3 Table type * (1 task each + 3 thread for each task)
executor = Executors.newFixedThreadPool(3 * (1 + TASK_THREAD_CNT));
OzoneManagerProtocolProtos.QuotaRepairRequest.Builder builder
= OzoneManagerProtocolProtos.QuotaRepairRequest.newBuilder();
// repair active db
- activeMetaManager = createActiveDBCheckpoint(om.getMetadataManager(), om.getConfiguration());
- repairActiveDb(activeMetaManager, builder, buckets);
+ try (OMMetadataManager activeMetaManager =
+ createActiveDBCheckpoint(om.getMetadataManager(), om.getConfiguration())) {
+ repairActiveDb(activeMetaManager, builder, buckets);
- // TODO: repair snapshots for quota
+ // TODO: repair snapshots for quota
Review Comment:
extra indentation over here as well.
--
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-13809. Close OmMetadataManagerImpl.createCheckpointMetadataManager properly [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on PR #9173:
URL: https://github.com/apache/ozone/pull/9173#issuecomment-3500749474
@chungen0126 I've fixed this PR following your suggestions.
--
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-13809. Close OmMetadataManagerImpl.createCheckpointMetadataManager properly [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on PR #9173:
URL: https://github.com/apache/ozone/pull/9173#issuecomment-3561321807
Thanks for the review @peterxcli !
--
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-13809. Close OmMetadataManagerImpl.createCheckpointMetadataManager properly [ozone]
Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on PR #9173:
URL: https://github.com/apache/ozone/pull/9173#issuecomment-3564090015
@rich7420 The CI failed. Please fix 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-13809. Close checkpoint OMMetadataManager properly [ozone]
Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on PR #9173:
URL: https://github.com/apache/ozone/pull/9173#issuecomment-3579913700
Thanks @rich7420 for the patch, @peterxcli @Gargi-jais11 for the review.
--
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-13809. Close OmMetadataManagerImpl.createCheckpointMetadataManager properly [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9173:
URL: https://github.com/apache/ozone/pull/9173#discussion_r2447451899
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServlet.java:
##########
@@ -349,12 +349,12 @@ private Set<Path> getSnapshotDirs(DBCheckpoint checkpoint, boolean waitForDir)
Set<Path> snapshotPaths = new HashSet<>();
// get snapshotInfo entries
- OmMetadataManagerImpl checkpointMetadataManager =
+ try (OmMetadataManagerImpl checkpointMetadataManager =
OmMetadataManagerImpl.createCheckpointMetadataManager(
conf, checkpoint);
- try (TableIterator<String, ? extends Table.KeyValue<String, SnapshotInfo>>
- iterator = checkpointMetadataManager
- .getSnapshotInfoTable().iterator()) {
+ TableIterator<String, ? extends Table.KeyValue<String, SnapshotInfo>>
+ iterator = checkpointMetadataManager
+ .getSnapshotInfoTable().iterator()) {
Review Comment:
Similarly over here.
--
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-13809. Close OmMetadataManagerImpl.createCheckpointMetadataManager properly [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #9173:
URL: https://github.com/apache/ozone/pull/9173#discussion_r2453956910
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/QuotaRepairTask.java:
##########
@@ -102,32 +102,33 @@ public static String getStatus() {
private boolean repairTask(List<String> buckets) {
LOG.info("Starting quota repair task {}", REPAIR_STATUS);
- OMMetadataManager activeMetaManager = null;
try {
// thread pool with 3 Table type * (1 task each + 3 thread for each task)
executor = Executors.newFixedThreadPool(3 * (1 + TASK_THREAD_CNT));
OzoneManagerProtocolProtos.QuotaRepairRequest.Builder builder
= OzoneManagerProtocolProtos.QuotaRepairRequest.newBuilder();
// repair active db
- activeMetaManager = createActiveDBCheckpoint(om.getMetadataManager(), om.getConfiguration());
- repairActiveDb(activeMetaManager, builder, buckets);
+ try (OMMetadataManager activeMetaManager =
+ createActiveDBCheckpoint(om.getMetadataManager(), om.getConfiguration())) {
+ repairActiveDb(activeMetaManager, builder, buckets);
// TODO: repair snapshots for quota
Review Comment:
this line should have indent, too.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ReconTaskControllerImpl.java:
##########
@@ -741,16 +740,16 @@ private void processReInitializationEvent(ReconTaskReInitializationEvent event)
resetRetryCounters();
LOG.info("Completed async task reinitialization");
}
+
+ // Clean up checkpoint files after use (database connections are automatically closed by try-with-resources)
+ cleanupCheckpointFiles(checkpointedOMMetadataManager);
Review Comment:
Could you elaborate why we need this new function? and should we remove the old one?
--
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-13809. Close OmMetadataManagerImpl.createCheckpointMetadataManager properly [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9173:
URL: https://github.com/apache/ozone/pull/9173#discussion_r2453965964
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ReconTaskControllerImpl.java:
##########
@@ -741,16 +740,16 @@ private void processReInitializationEvent(ReconTaskReInitializationEvent event)
resetRetryCounters();
LOG.info("Completed async task reinitialization");
}
+
+ // Clean up checkpoint files after use (database connections are automatically closed by try-with-resources)
+ cleanupCheckpointFiles(checkpointedOMMetadataManager);
Review Comment:
I was also wondering the same thing as there is only one line of difference between the two functions.
```
// Close the database connections first
checkpointedManager.close();
LOG.debug("Closed checkpointed OM metadata manager database connections");
```
--
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-13809. Close OmMetadataManagerImpl.createCheckpointMetadataManager properly [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9173:
URL: https://github.com/apache/ozone/pull/9173#discussion_r2447430859
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServlet.java:
##########
@@ -349,12 +349,12 @@ private Set<Path> getSnapshotDirs(DBCheckpoint checkpoint, boolean waitForDir)
Set<Path> snapshotPaths = new HashSet<>();
// get snapshotInfo entries
- OmMetadataManagerImpl checkpointMetadataManager =
+ try (OmMetadataManagerImpl checkpointMetadataManager =
OmMetadataManagerImpl.createCheckpointMetadataManager(
conf, checkpoint);
- try (TableIterator<String, ? extends Table.KeyValue<String, SnapshotInfo>>
- iterator = checkpointMetadataManager
- .getSnapshotInfoTable().iterator()) {
+ TableIterator<String, ? extends Table.KeyValue<String, SnapshotInfo>>
Review Comment:
Extra indentation has been added in multiple places, please revert these to match the original formatting.
--
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-13809. Close OmMetadataManagerImpl.createCheckpointMetadataManager properly [ozone]
Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on code in PR #9173:
URL: https://github.com/apache/ozone/pull/9173#discussion_r2501512395
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ReconTaskControllerImpl.java:
##########
@@ -741,16 +744,16 @@ private void processReInitializationEvent(ReconTaskReInitializationEvent event)
resetRetryCounters();
LOG.info("Completed async task reinitialization");
}
+
+ // Clean up checkpoint files after use (database connections are automatically closed by try-with-resources)
+ cleanupCheckpointFiles(checkpointedOMMetadataManager);
Review Comment:
It's good to separate the file cleanup and the closing of `ReconOmMetadataManagerImpl`, but the file cleanup should still be put in a finally block to ensure it runs properly.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/QuotaRepairTask.java:
##########
@@ -102,32 +102,33 @@ public static String getStatus() {
private boolean repairTask(List<String> buckets) {
LOG.info("Starting quota repair task {}", REPAIR_STATUS);
- OMMetadataManager activeMetaManager = null;
try {
// thread pool with 3 Table type * (1 task each + 3 thread for each task)
executor = Executors.newFixedThreadPool(3 * (1 + TASK_THREAD_CNT));
OzoneManagerProtocolProtos.QuotaRepairRequest.Builder builder
= OzoneManagerProtocolProtos.QuotaRepairRequest.newBuilder();
// repair active db
- activeMetaManager = createActiveDBCheckpoint(om.getMetadataManager(), om.getConfiguration());
- repairActiveDb(activeMetaManager, builder, buckets);
+ try (OMMetadataManager activeMetaManager =
Review Comment:
There are too many nested try blocks. Could we declare the active DB checkpoint in the existing try-with-resources?
--
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-13809. Close OmMetadataManagerImpl.createCheckpointMetadataManager properly [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on PR #9173:
URL: https://github.com/apache/ozone/pull/9173#issuecomment-3425669424
@Gargi-jais11 Thanks for feedback!
--
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-13809. Close OmMetadataManagerImpl.createCheckpointMetadataManager properly [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on code in PR #9173:
URL: https://github.com/apache/ozone/pull/9173#discussion_r2450658640
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/QuotaRepairTask.java:
##########
@@ -102,32 +102,33 @@ public static String getStatus() {
private boolean repairTask(List<String> buckets) {
LOG.info("Starting quota repair task {}", REPAIR_STATUS);
- OMMetadataManager activeMetaManager = null;
try {
// thread pool with 3 Table type * (1 task each + 3 thread for each task)
executor = Executors.newFixedThreadPool(3 * (1 + TASK_THREAD_CNT));
OzoneManagerProtocolProtos.QuotaRepairRequest.Builder builder
= OzoneManagerProtocolProtos.QuotaRepairRequest.newBuilder();
// repair active db
- activeMetaManager = createActiveDBCheckpoint(om.getMetadataManager(), om.getConfiguration());
- repairActiveDb(activeMetaManager, builder, buckets);
+ try (OMMetadataManager activeMetaManager =
+ createActiveDBCheckpoint(om.getMetadataManager(), om.getConfiguration())) {
+ repairActiveDb(activeMetaManager, builder, buckets);
- // TODO: repair snapshots for quota
+ // TODO: repair snapshots for quota
Review Comment:
OK!
--
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-13809. Close OmMetadataManagerImpl.createCheckpointMetadataManager properly [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on code in PR #9173:
URL: https://github.com/apache/ozone/pull/9173#discussion_r2553753036
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/QuotaRepairTask.java:
##########
@@ -102,14 +102,13 @@ public static String getStatus() {
private boolean repairTask(List<String> buckets) {
LOG.info("Starting quota repair task {}", REPAIR_STATUS);
- OMMetadataManager activeMetaManager = null;
- try {
- // thread pool with 3 Table type * (1 task each + 3 thread for each task)
- executor = Executors.newFixedThreadPool(3 * (1 + TASK_THREAD_CNT));
+ // thread pool with 3 Table type * (1 task each + 3 thread for each task)
+ executor = Executors.newFixedThreadPool(3 * (1 + TASK_THREAD_CNT));
+ try (OMMetadataManager activeMetaManager =
Review Comment:
sorry @peterxcli , this change will get error. I think maybe is because executor is not AutoCloseable so we couldn't put it in try-with-resources.
--
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-13809. Close OmMetadataManagerImpl.createCheckpointMetadataManager properly [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #9173:
URL: https://github.com/apache/ozone/pull/9173#discussion_r2546308399
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ReconTaskControllerImpl.java:
##########
@@ -749,8 +758,10 @@ private void processReInitializationEvent(ReconTaskReInitializationEvent event)
} catch (Exception e) {
LOG.error("Error processing reinitialization event", e);
} finally {
- // Clean up the checkpointed metadata manager and its files after use
- cleanupCheckpoint(checkpointedOMMetadataManager);
+ // Clean up checkpoint files after use (database connections are automatically closed by try-with-resources)
Review Comment:
```suggestion
```
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ReconTaskControllerImpl.java:
##########
@@ -749,8 +758,10 @@ private void processReInitializationEvent(ReconTaskReInitializationEvent event)
} catch (Exception e) {
LOG.error("Error processing reinitialization event", e);
} finally {
- // Clean up the checkpointed metadata manager and its files after use
- cleanupCheckpoint(checkpointedOMMetadataManager);
+ // Clean up checkpoint files after use (database connections are automatically closed by try-with-resources)
Review Comment:
this comment is a little redundant
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/QuotaRepairTask.java:
##########
@@ -102,14 +102,13 @@ public static String getStatus() {
private boolean repairTask(List<String> buckets) {
LOG.info("Starting quota repair task {}", REPAIR_STATUS);
- OMMetadataManager activeMetaManager = null;
- try {
- // thread pool with 3 Table type * (1 task each + 3 thread for each task)
- executor = Executors.newFixedThreadPool(3 * (1 + TASK_THREAD_CNT));
+ // thread pool with 3 Table type * (1 task each + 3 thread for each task)
+ executor = Executors.newFixedThreadPool(3 * (1 + TASK_THREAD_CNT));
+ try (OMMetadataManager activeMetaManager =
Review Comment:
```suggestion
try (OMMetadataManager activeMetaManager =
executor = Executors.newFixedThreadPool(3 * (1 + TASK_THREAD_CNT));
```
--
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-13809. Close checkpoint OMMetadataManager properly [ozone]
Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 merged PR #9173:
URL: https://github.com/apache/ozone/pull/9173
--
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-13809. Close OmMetadataManagerImpl.createCheckpointMetadataManager properly [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on code in PR #9173:
URL: https://github.com/apache/ozone/pull/9173#discussion_r2454224080
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ReconTaskControllerImpl.java:
##########
@@ -741,16 +740,16 @@ private void processReInitializationEvent(ReconTaskReInitializationEvent event)
resetRetryCounters();
LOG.info("Completed async task reinitialization");
}
+
+ // Clean up checkpoint files after use (database connections are automatically closed by try-with-resources)
+ cleanupCheckpointFiles(checkpointedOMMetadataManager);
Review Comment:
@peterxcli , @Gargi-jais11 you're right that's a unnecessary new function. I've removed 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-13809. Close OmMetadataManagerImpl.createCheckpointMetadataManager properly [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on code in PR #9173:
URL: https://github.com/apache/ozone/pull/9173#discussion_r2454221349
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/QuotaRepairTask.java:
##########
@@ -102,32 +102,33 @@ public static String getStatus() {
private boolean repairTask(List<String> buckets) {
LOG.info("Starting quota repair task {}", REPAIR_STATUS);
- OMMetadataManager activeMetaManager = null;
try {
// thread pool with 3 Table type * (1 task each + 3 thread for each task)
executor = Executors.newFixedThreadPool(3 * (1 + TASK_THREAD_CNT));
OzoneManagerProtocolProtos.QuotaRepairRequest.Builder builder
= OzoneManagerProtocolProtos.QuotaRepairRequest.newBuilder();
// repair active db
- activeMetaManager = createActiveDBCheckpoint(om.getMetadataManager(), om.getConfiguration());
- repairActiveDb(activeMetaManager, builder, buckets);
+ try (OMMetadataManager activeMetaManager =
+ createActiveDBCheckpoint(om.getMetadataManager(), om.getConfiguration())) {
+ repairActiveDb(activeMetaManager, builder, buckets);
// TODO: repair snapshots for quota
Review Comment:
Thanks for the feedback!
--
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-13809. Close OmMetadataManagerImpl.createCheckpointMetadataManager properly [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on PR #9173:
URL: https://github.com/apache/ozone/pull/9173#issuecomment-3500556016
thanks @chungen0126 for your feedback! I'll try to fix it as soon as possible.
--
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-13809. Close OmMetadataManagerImpl.createCheckpointMetadataManager properly [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on PR #9173:
URL: https://github.com/apache/ozone/pull/9173#issuecomment-3567449840
@chungen0126 Thanks for the reminding!
--
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-13809. Close checkpoint OMMetadataManager properly [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #9173:
URL: https://github.com/apache/ozone/pull/9173#issuecomment-3579963851
@chungen0126 when merging PRs, `Co-authored-by` can be deleted for those who only did branch merge (me in this case).
--
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