sumitagrawl commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1828806682
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
@@ -102,6 +104,16 @@ public DirectoryDeletingService(long interval, TimeUnit
unit,
this.ratisByteLimit = (int) (limit * 0.9);
this.suspended = new AtomicBoolean(false);
this.isRunningOnAOS = new AtomicBoolean(false);
+ this.dirDeletingCorePoolSize = dirDeletingServiceCorePoolSize;
+ try {
+ deletedDirSupplier = new DeletedDirSupplier();
+ } catch (IOException ex) {
+ LOG.error("Couldn't initialize supplier.");
Review Comment:
we can avoid exception, init on need basis, not in constructor for iterator
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
@@ -135,10 +147,69 @@ public void resume() {
@Override
public BackgroundTaskQueue getTasks() {
BackgroundTaskQueue queue = new BackgroundTaskQueue();
- queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+ deletedDirSupplier.reInitItr();
+ for (int i = 0; i < dirDeletingCorePoolSize; i++) {
+ queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+ }
return queue;
}
+ @Override
+ public void shutdown() {
+ super.shutdown();
+ deletedDirSupplier.closeItr();
+ }
+
+ private final class DeletedDirSupplier {
+ private TableIterator<String, ? extends KeyValue<String, OmKeyInfo>>
+ deleteTableIterator;
+
+ private DeletedDirSupplier() throws IOException {
+ this.deleteTableIterator =
+ getOzoneManager().getMetadataManager().getDeletedDirTable()
+ .iterator();
+ }
+
+ private TableIterator<String, ? extends KeyValue<String, OmKeyInfo>>
getDeleteTableIterator() {
+ return deleteTableIterator;
+ }
+
+ private synchronized Table.KeyValue<String, OmKeyInfo> get() {
+ if (deleteTableIterator != null && !deleteTableIterator.hasNext()) {
+ try {
+ if (getOzoneManager().getMetadataManager().getDeletedDirTable()
+ .isEmpty()) {
+ closeItr();
+ } else {
+ reInitItr();
+ }
+ } catch (IOException ex) {
+ LOG.error("Not able to determine if deleted dir table is empty.");
Review Comment:
need throw exception if unable to get data.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
@@ -135,10 +147,69 @@ public void resume() {
@Override
public BackgroundTaskQueue getTasks() {
BackgroundTaskQueue queue = new BackgroundTaskQueue();
- queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+ deletedDirSupplier.reInitItr();
+ for (int i = 0; i < dirDeletingCorePoolSize; i++) {
+ queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+ }
return queue;
}
+ @Override
+ public void shutdown() {
+ super.shutdown();
+ deletedDirSupplier.closeItr();
+ }
+
+ private final class DeletedDirSupplier {
+ private TableIterator<String, ? extends KeyValue<String, OmKeyInfo>>
+ deleteTableIterator;
+
+ private DeletedDirSupplier() throws IOException {
+ this.deleteTableIterator =
+ getOzoneManager().getMetadataManager().getDeletedDirTable()
+ .iterator();
+ }
+
+ private TableIterator<String, ? extends KeyValue<String, OmKeyInfo>>
getDeleteTableIterator() {
+ return deleteTableIterator;
+ }
+
+ private synchronized Table.KeyValue<String, OmKeyInfo> get() {
+ if (deleteTableIterator != null && !deleteTableIterator.hasNext()) {
+ try {
+ if (getOzoneManager().getMetadataManager().getDeletedDirTable()
+ .isEmpty()) {
+ closeItr();
+ } else {
+ reInitItr();
+ }
+ } catch (IOException ex) {
+ LOG.error("Not able to determine if deleted dir table is empty.");
+ }
+ }
+ return deleteTableIterator != null ? deleteTableIterator.next() : null;
Review Comment:
need check if hasNext(), then return next(), else null
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
@@ -135,10 +147,69 @@ public void resume() {
@Override
public BackgroundTaskQueue getTasks() {
BackgroundTaskQueue queue = new BackgroundTaskQueue();
- queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+ deletedDirSupplier.reInitItr();
+ for (int i = 0; i < dirDeletingCorePoolSize; i++) {
+ queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+ }
return queue;
}
+ @Override
+ public void shutdown() {
+ super.shutdown();
+ deletedDirSupplier.closeItr();
+ }
+
+ private final class DeletedDirSupplier {
+ private TableIterator<String, ? extends KeyValue<String, OmKeyInfo>>
+ deleteTableIterator;
+
+ private DeletedDirSupplier() throws IOException {
+ this.deleteTableIterator =
+ getOzoneManager().getMetadataManager().getDeletedDirTable()
+ .iterator();
+ }
+
+ private TableIterator<String, ? extends KeyValue<String, OmKeyInfo>>
getDeleteTableIterator() {
+ return deleteTableIterator;
+ }
+
+ private synchronized Table.KeyValue<String, OmKeyInfo> get() {
+ if (deleteTableIterator != null && !deleteTableIterator.hasNext()) {
+ try {
+ if (getOzoneManager().getMetadataManager().getDeletedDirTable()
+ .isEmpty()) {
+ closeItr();
+ } else {
+ reInitItr();
+ }
+ } catch (IOException ex) {
+ LOG.error("Not able to determine if deleted dir table is empty.");
+ }
+ }
+ return deleteTableIterator != null ? deleteTableIterator.next() : null;
+ }
+
+ private void closeItr() {
+ try {
+ deleteTableIterator.close();
+ } catch (IOException ex) {
+ LOG.error("Couldn't close the iterator ", ex);
+ }
+ }
+
+ private void reInitItr() {
+ try {
+ deleteTableIterator.close();
Review Comment:
if not null, then only close
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
@@ -135,10 +147,69 @@ public void resume() {
@Override
public BackgroundTaskQueue getTasks() {
BackgroundTaskQueue queue = new BackgroundTaskQueue();
- queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+ deletedDirSupplier.reInitItr();
Review Comment:
can avoid reinit iterator here, can be done on need bases, like initially
null. In get(), if null or or hasNext is false, can init there.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
@@ -135,10 +147,69 @@ public void resume() {
@Override
public BackgroundTaskQueue getTasks() {
BackgroundTaskQueue queue = new BackgroundTaskQueue();
- queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+ deletedDirSupplier.reInitItr();
+ for (int i = 0; i < dirDeletingCorePoolSize; i++) {
+ queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+ }
return queue;
}
+ @Override
+ public void shutdown() {
+ super.shutdown();
+ deletedDirSupplier.closeItr();
+ }
+
+ private final class DeletedDirSupplier {
+ private TableIterator<String, ? extends KeyValue<String, OmKeyInfo>>
+ deleteTableIterator;
+
+ private DeletedDirSupplier() throws IOException {
+ this.deleteTableIterator =
+ getOzoneManager().getMetadataManager().getDeletedDirTable()
+ .iterator();
+ }
+
+ private TableIterator<String, ? extends KeyValue<String, OmKeyInfo>>
getDeleteTableIterator() {
+ return deleteTableIterator;
+ }
+
+ private synchronized Table.KeyValue<String, OmKeyInfo> get() {
+ if (deleteTableIterator != null && !deleteTableIterator.hasNext()) {
+ try {
+ if (getOzoneManager().getMetadataManager().getDeletedDirTable()
+ .isEmpty()) {
+ closeItr();
+ } else {
+ reInitItr();
+ }
+ } catch (IOException ex) {
+ LOG.error("Not able to determine if deleted dir table is empty.");
+ }
+ }
+ return deleteTableIterator != null ? deleteTableIterator.next() : null;
+ }
+
+ private void closeItr() {
+ try {
+ deleteTableIterator.close();
Review Comment:
set to null after close to indicate its already closed.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
@@ -135,10 +147,69 @@ public void resume() {
@Override
public BackgroundTaskQueue getTasks() {
BackgroundTaskQueue queue = new BackgroundTaskQueue();
- queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+ deletedDirSupplier.reInitItr();
+ for (int i = 0; i < dirDeletingCorePoolSize; i++) {
+ queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+ }
return queue;
}
+ @Override
+ public void shutdown() {
+ super.shutdown();
+ deletedDirSupplier.closeItr();
+ }
+
+ private final class DeletedDirSupplier {
+ private TableIterator<String, ? extends KeyValue<String, OmKeyInfo>>
+ deleteTableIterator;
+
+ private DeletedDirSupplier() throws IOException {
+ this.deleteTableIterator =
+ getOzoneManager().getMetadataManager().getDeletedDirTable()
+ .iterator();
+ }
+
+ private TableIterator<String, ? extends KeyValue<String, OmKeyInfo>>
getDeleteTableIterator() {
+ return deleteTableIterator;
+ }
+
+ private synchronized Table.KeyValue<String, OmKeyInfo> get() {
+ if (deleteTableIterator != null && !deleteTableIterator.hasNext()) {
+ try {
+ if (getOzoneManager().getMetadataManager().getDeletedDirTable()
+ .isEmpty()) {
+ closeItr();
Review Comment:
need return null if no element
--
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-unsubscr...@ozone.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@ozone.apache.org
For additional commands, e-mail: issues-h...@ozone.apache.org
