Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
sumitagrawl merged PR #7349: URL: https://github.com/apache/ozone/pull/7349 -- 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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
aryangupta1998 commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1870848228
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -135,10 +148,54 @@ public void resume() {
@Override
public BackgroundTaskQueue getTasks() {
BackgroundTaskQueue queue = new BackgroundTaskQueue();
-queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+if (taskCount.get() > 0) {
+ LOG.info("{} Directory deleting task(s) already in progress.",
+ taskCount.get());
+ return queue;
+}
+try {
+ deletedDirSupplier.reInitItr();
+} catch (IOException ex) {
+ LOG.error("Unable to get the iterator.", ex);
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-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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
aryangupta1998 commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1870847828
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -158,7 +215,8 @@ public BackgroundTaskResult call() {
LOG.debug("Running DirectoryDeletingService");
}
isRunningOnAOS.set(true);
-getRunCount().incrementAndGet();
+// rnCnt will be same for each thread to maintain same CallId.
+long rnCnt = getRunCount().incrementAndGet();
Review Comment:
Removed comment as discussed.
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -238,6 +295,7 @@ public BackgroundTaskResult call() {
}
}
// place holder by returning empty results of this call back.
+ taskCount.getAndDecrement();
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-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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
ashishkumar50 commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1870810793
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -238,6 +295,7 @@ public BackgroundTaskResult call() {
}
}
// place holder by returning empty results of this call back.
+ taskCount.getAndDecrement();
Review Comment:
Better to do `taskCount.getAndDecrement();` in finally. In case of some
exception it doesn't get decremented then from next time complete
DeletingService will halt.
--
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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
ashishkumar50 commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1870779874
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -158,7 +215,8 @@ public BackgroundTaskResult call() {
LOG.debug("Running DirectoryDeletingService");
}
isRunningOnAOS.set(true);
-getRunCount().incrementAndGet();
+// rnCnt will be same for each thread to maintain same CallId.
+long rnCnt = getRunCount().incrementAndGet();
Review Comment:
Comment looks wrong. rnCnt is different for each request/thread and so
CallId will be different which is correct.
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -135,10 +148,54 @@ public void resume() {
@Override
public BackgroundTaskQueue getTasks() {
BackgroundTaskQueue queue = new BackgroundTaskQueue();
-queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+if (taskCount.get() > 0) {
+ LOG.info("{} Directory deleting task(s) already in progress.",
+ taskCount.get());
+ return queue;
+}
+try {
+ deletedDirSupplier.reInitItr();
+} catch (IOException ex) {
+ LOG.error("Unable to get the iterator.", ex);
Review Comment:
In case of exception we don't need to continue to next statements, instead
we need to return here.
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -238,6 +295,7 @@ public BackgroundTaskResult call() {
}
}
// place holder by returning empty results of this call back.
+ taskCount.getAndDecrement();
Review Comment:
Better to do `taskCount.getAndDecrement();` in finally. I some en case of
some exception it doesn't get decremented then from next time complete
deletingservice will halt.
--
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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
aryangupta1998 commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1870707298
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -135,10 +148,54 @@ public void resume() {
@Override
public BackgroundTaskQueue getTasks() {
BackgroundTaskQueue queue = new BackgroundTaskQueue();
-queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+if (taskCount.get() > 0) {
Review Comment:
Thanks, it shouldn't make much of a performance difference, as per the
default thread config(10) and limit per task(6000), all threads should be able
to complete within the deletion interval.
--
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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
sadanand48 commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1870700483
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -135,10 +148,54 @@ public void resume() {
@Override
public BackgroundTaskQueue getTasks() {
BackgroundTaskQueue queue = new BackgroundTaskQueue();
-queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+if (taskCount.get() > 0) {
Review Comment:
Looks okay, just wanted to highlight this point.
--
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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
aryangupta1998 commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1870696441
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -135,10 +148,54 @@ public void resume() {
@Override
public BackgroundTaskQueue getTasks() {
BackgroundTaskQueue queue = new BackgroundTaskQueue();
-queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+if (taskCount.get() > 0) {
Review Comment:
Yes.
--
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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
sadanand48 commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1870695330
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -135,10 +148,54 @@ public void resume() {
@Override
public BackgroundTaskQueue getTasks() {
BackgroundTaskQueue queue = new BackgroundTaskQueue();
-queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+if (taskCount.get() > 0) {
Review Comment:
This means until current tasks are completed, new tasks won't be issued even
if there are few idle threads?
--
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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
aryangupta1998 commented on PR #7349: URL: https://github.com/apache/ozone/pull/7349#issuecomment-2474466981 ### Performance Test Results: Directory Deletion I tested the deletion of **200,000** directories in a multi-level structure under various configurations. Below are my observations: Configuration: Path limit per task: 7,000 Ratis log appender: 32 MB Deletion interval: 3 seconds Threads: 10 threads **Single-threaded vs Multi-threaded Performance:** **Single-threaded (1 thread):** Time taken: 15 minutes Directories deleted: 45,000 directories **Multi-threaded (10 threads):** Time taken: 7 minutes Directories deleted: 200,000 directories Performance improvement: Achieved nearly 10x faster deletion with multi-threading. **Testing with More Threads:** I also tested with 30 threads and 50 threads, keeping other configurations the same. With 30 threads: Time taken: 3 minutes Directories deleted: 200,000 directories With 50 threads: Time taken: 1 minute 50 seconds Directories deleted: 200,000 directories -- 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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
sumitagrawl commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1833721607
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -169,27 +210,27 @@ public BackgroundTaskResult call() {
= new ArrayList<>((int) remainNum);
Table.KeyValue pendingDeletedDirInfo;
-
-try (TableIterator>
- deleteTableIterator = getOzoneManager().getMetadataManager().
-getDeletedDirTable().iterator()) {
- // This is to avoid race condition b/w purge request and snapshot
chain updation. For AOS taking the global
- // snapshotId since AOS could process multiple buckets in one
iteration.
+// This is to avoid race condition b/w purge request and snapshot
chain updation. For AOS taking the global
+// snapshotId since AOS could process multiple buckets in one
iteration.
+try {
UUID expectedPreviousSnapshotId =
-
((OmMetadataManagerImpl)getOzoneManager().getMetadataManager()).getSnapshotChainManager()
+ ((OmMetadataManagerImpl)
getOzoneManager().getMetadataManager()).getSnapshotChainManager()
.getLatestGlobalSnapshotId();
long startTime = Time.monotonicNow();
- while (remainNum > 0 && deleteTableIterator.hasNext()) {
-pendingDeletedDirInfo = deleteTableIterator.next();
+ while (remainNum > 0) {
+pendingDeletedDirInfo = deletedDirSupplier.get();
Review Comment:
There is a concern of getting 10K batches, mostly it will be wasted as per
logic,
- Get parent
-- scan subdir and subfile
-- if subdir/subfile itself meet the limit, then it will not read remaining
9.999k records in this iteration.
Limit: ratis size OR limit being configured, and this limit is per thread.
So its suggested not to have cache. Additionally, multiple iterator have
concern of duplicate reading or managing gaps between threads.
So We can test, various scenario if iteration can be completed in a minute.
1) 10 threads, each thread limit: 10k, flat director only within a parent
and count: 200k
2) 50 threads, each limit: 10k, flat directory only within a parent and
count: 2M
3) 10 thread, each limit 50k, flat directory only within a parent and count:
2M
This can check comparision of 2 and 3 case, with info log for time taken as
already log present.
cc: @aryangupta1998
--
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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
errose28 commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1831345088
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -169,27 +210,27 @@ public BackgroundTaskResult call() {
= new ArrayList<>((int) remainNum);
Table.KeyValue pendingDeletedDirInfo;
-
-try (TableIterator>
- deleteTableIterator = getOzoneManager().getMetadataManager().
-getDeletedDirTable().iterator()) {
- // This is to avoid race condition b/w purge request and snapshot
chain updation. For AOS taking the global
- // snapshotId since AOS could process multiple buckets in one
iteration.
+// This is to avoid race condition b/w purge request and snapshot
chain updation. For AOS taking the global
+// snapshotId since AOS could process multiple buckets in one
iteration.
+try {
UUID expectedPreviousSnapshotId =
-
((OmMetadataManagerImpl)getOzoneManager().getMetadataManager()).getSnapshotChainManager()
+ ((OmMetadataManagerImpl)
getOzoneManager().getMetadataManager()).getSnapshotChainManager()
.getLatestGlobalSnapshotId();
long startTime = Time.monotonicNow();
- while (remainNum > 0 && deleteTableIterator.hasNext()) {
-pendingDeletedDirInfo = deleteTableIterator.next();
+ while (remainNum > 0) {
+pendingDeletedDirInfo = deletedDirSupplier.get();
Review Comment:
I think we can micro-benchmark this to test and be sure which is the best
approach. If there's no speedup to retrieving batches of 10,100, etc per get
then +1 for keeping it simple with a single get, but I think something like
this we can test out to get a conclusive answer.
--
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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
sumitagrawl commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1830597235
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -169,27 +210,27 @@ public BackgroundTaskResult call() {
= new ArrayList<>((int) remainNum);
Table.KeyValue pendingDeletedDirInfo;
-
-try (TableIterator>
- deleteTableIterator = getOzoneManager().getMetadataManager().
-getDeletedDirTable().iterator()) {
- // This is to avoid race condition b/w purge request and snapshot
chain updation. For AOS taking the global
- // snapshotId since AOS could process multiple buckets in one
iteration.
+// This is to avoid race condition b/w purge request and snapshot
chain updation. For AOS taking the global
+// snapshotId since AOS could process multiple buckets in one
iteration.
+try {
UUID expectedPreviousSnapshotId =
-
((OmMetadataManagerImpl)getOzoneManager().getMetadataManager()).getSnapshotChainManager()
+ ((OmMetadataManagerImpl)
getOzoneManager().getMetadataManager()).getSnapshotChainManager()
.getLatestGlobalSnapshotId();
long startTime = Time.monotonicNow();
- while (remainNum > 0 && deleteTableIterator.hasNext()) {
-pendingDeletedDirInfo = deleteTableIterator.next();
+ while (remainNum > 0) {
+pendingDeletedDirInfo = deletedDirSupplier.get();
Review Comment:
IMO,
get() will be synchronized for 1 unit of time, and processing will take 5-10
unit of time minimum as, it processing involves,
- search in file table
- search in directory table
- prepare request
So this contention will not have much impact in processing with default 10
threads. Just keep simple instead of having cache / or other mechanism.
--
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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
ashishkumar50 commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1830388634
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -169,27 +210,27 @@ public BackgroundTaskResult call() {
= new ArrayList<>((int) remainNum);
Table.KeyValue pendingDeletedDirInfo;
-
-try (TableIterator>
- deleteTableIterator = getOzoneManager().getMetadataManager().
-getDeletedDirTable().iterator()) {
- // This is to avoid race condition b/w purge request and snapshot
chain updation. For AOS taking the global
- // snapshotId since AOS could process multiple buckets in one
iteration.
+// This is to avoid race condition b/w purge request and snapshot
chain updation. For AOS taking the global
+// snapshotId since AOS could process multiple buckets in one
iteration.
+try {
UUID expectedPreviousSnapshotId =
-
((OmMetadataManagerImpl)getOzoneManager().getMetadataManager()).getSnapshotChainManager()
+ ((OmMetadataManagerImpl)
getOzoneManager().getMetadataManager()).getSnapshotChainManager()
.getLatestGlobalSnapshotId();
long startTime = Time.monotonicNow();
- while (remainNum > 0 && deleteTableIterator.hasNext()) {
-pendingDeletedDirInfo = deleteTableIterator.next();
+ while (remainNum > 0) {
+pendingDeletedDirInfo = deletedDirSupplier.get();
Review Comment:
Can't we get multiple pending directories to delete here, so that to
decrease probability of all threads to wait for `synchronized get` at the same
time.
--
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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
ashishkumar50 commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1830388634
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -169,27 +210,27 @@ public BackgroundTaskResult call() {
= new ArrayList<>((int) remainNum);
Table.KeyValue pendingDeletedDirInfo;
-
-try (TableIterator>
- deleteTableIterator = getOzoneManager().getMetadataManager().
-getDeletedDirTable().iterator()) {
- // This is to avoid race condition b/w purge request and snapshot
chain updation. For AOS taking the global
- // snapshotId since AOS could process multiple buckets in one
iteration.
+// This is to avoid race condition b/w purge request and snapshot
chain updation. For AOS taking the global
+// snapshotId since AOS could process multiple buckets in one
iteration.
+try {
UUID expectedPreviousSnapshotId =
-
((OmMetadataManagerImpl)getOzoneManager().getMetadataManager()).getSnapshotChainManager()
+ ((OmMetadataManagerImpl)
getOzoneManager().getMetadataManager()).getSnapshotChainManager()
.getLatestGlobalSnapshotId();
long startTime = Time.monotonicNow();
- while (remainNum > 0 && deleteTableIterator.hasNext()) {
-pendingDeletedDirInfo = deleteTableIterator.next();
+ while (remainNum > 0) {
+pendingDeletedDirInfo = deletedDirSupplier.get();
Review Comment:
Can we get multiple pending directories to delete here, so that to decrease
probability of all threads to wait for `synchronized get` at the same time.
--
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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
adoroszlai commented on PR #7349: URL: https://github.com/apache/ozone/pull/7349#issuecomment-2456428605 Please wait for clean CI run in fork before marking "ready for 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-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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
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>
+deleteTableIterator;
+
+private DeletedDirSupplier() throws IOException {
+ this.deleteTableIterator =
+ getOzoneManager().getMetadataManager().getDeletedDirTable()
+ .iterator();
+}
+
+private TableIterator>
getDeleteTableIterator() {
+ return deleteTableIterator;
+}
+
+private synchronized Table.KeyValue 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>
+deleteTableIterator;
+
+private DeletedDirSupplier() throws IOException {
+ this.deleteTableIterator =
+ getOzoneManager().getMetadataManager().getDeletedDirTable()
+ .iterator();
+}
+
+private TableIterator>
getDeleteTableIterator() {
+ return deleteTableIterator;
+}
+
+private synchronized Table.KeyValue 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>
+deleteTableIterator;
+
+private DeletedDirSupplier() throws IOException {
+ this.deleteTableIterator =
+ getOzoneManager().getMetadataManager().getDeletedDirTable()
+ .iterator(
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
aryangupta1998 commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1828240755
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -169,27 +231,24 @@ public BackgroundTaskResult call() {
= new ArrayList<>((int) remainNum);
Table.KeyValue pendingDeletedDirInfo;
-
-try (TableIterator>
- deleteTableIterator = getOzoneManager().getMetadataManager().
-getDeletedDirTable().iterator()) {
- // This is to avoid race condition b/w purge request and snapshot
chain updation. For AOS taking the global
- // snapshotId since AOS could process multiple buckets in one
iteration.
+// This is to avoid race condition b/w purge request and snapshot
chain updation. For AOS taking the global
+// snapshotId since AOS could process multiple buckets in one
iteration.
+try {
UUID expectedPreviousSnapshotId =
-
((OmMetadataManagerImpl)getOzoneManager().getMetadataManager()).getSnapshotChainManager()
+ ((OmMetadataManagerImpl)
getOzoneManager().getMetadataManager()).getSnapshotChainManager()
.getLatestGlobalSnapshotId();
long startTime = Time.monotonicNow();
- while (remainNum > 0 && deleteTableIterator.hasNext()) {
-pendingDeletedDirInfo = deleteTableIterator.next();
+ while (remainNum > 0) {
+pendingDeletedDirInfo = supplier.getItr();
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-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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
aryangupta1998 commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1828239238
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -77,20 +80,21 @@ public class DirectoryDeletingService extends
AbstractKeyDeletingService {
// Use only a single thread for DirDeletion. Multiple threads would read
// or write to same tables and can send deletion requests for same key
// multiple times.
- private static final int DIR_DELETING_CORE_POOL_SIZE = 1;
+ private static int DIR_DELETING_CORE_POOL_SIZE;
Review Comment:
Removed static keyword.
--
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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
aryangupta1998 commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1828240424
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -135,10 +142,65 @@ public void resume() {
@Override
public BackgroundTaskQueue getTasks() {
BackgroundTaskQueue queue = new BackgroundTaskQueue();
-queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+supplier.reInitItr();
+for (int i = 0; i < DIR_DELETING_CORE_POOL_SIZE; i++) {
+ queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+}
return queue;
}
+ @Override
+ public void shutdown() {
+super.shutdown();
+supplier.closeItr();
+ }
+
+ private final class Supplier {
+TableIterator>
+deleteTableIterator;
+
+private Supplier() throws IOException {
+ this.deleteTableIterator =
+ getOzoneManager().getMetadataManager().getDeletedDirTable()
+ .iterator();
+}
+
+private synchronized Table.KeyValue getItr() {
+ if (deleteTableIterator != null && !deleteTableIterator.hasNext()) {
+try {
+ if (getOzoneManager().getMetadataManager().getDeletedDirTable()
Review Comment:
Done
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -135,10 +142,65 @@ public void resume() {
@Override
public BackgroundTaskQueue getTasks() {
BackgroundTaskQueue queue = new BackgroundTaskQueue();
-queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+supplier.reInitItr();
+for (int i = 0; i < DIR_DELETING_CORE_POOL_SIZE; i++) {
+ queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+}
return queue;
}
+ @Override
+ public void shutdown() {
+super.shutdown();
+supplier.closeItr();
+ }
+
+ private final class Supplier {
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-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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
aryangupta1998 commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1828240119
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -135,10 +142,65 @@ public void resume() {
@Override
public BackgroundTaskQueue getTasks() {
BackgroundTaskQueue queue = new BackgroundTaskQueue();
-queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+supplier.reInitItr();
+for (int i = 0; i < DIR_DELETING_CORE_POOL_SIZE; i++) {
+ queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+}
return queue;
}
+ @Override
+ public void shutdown() {
+super.shutdown();
+supplier.closeItr();
+ }
+
+ private final class Supplier {
+TableIterator>
+deleteTableIterator;
+
+private Supplier() throws IOException {
+ this.deleteTableIterator =
+ getOzoneManager().getMetadataManager().getDeletedDirTable()
+ .iterator();
+}
+
+private synchronized Table.KeyValue getItr() {
Review Comment:
Renamed to get()
--
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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
aryangupta1998 commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1828239605
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##
@@ -257,8 +259,13 @@ public void start(OzoneConfiguration configuration) {
OZONE_BLOCK_DELETING_SERVICE_TIMEOUT,
OZONE_BLOCK_DELETING_SERVICE_TIMEOUT_DEFAULT,
TimeUnit.MILLISECONDS);
- dirDeletingService = new DirectoryDeletingService(dirDeleteInterval,
- TimeUnit.MILLISECONDS, serviceTimeout, ozoneManager, configuration);
+ int dirDeletingServiceCorePoolSize =
+ configuration.getInt(OZONE_THREAD_NUMBER_DIR_DELETION,
+ OZONE_THREAD_NUMBER_DIR_DELETION_DEFAULT);
+ dirDeletingService =
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-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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
aryangupta1998 commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1828238760
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -77,20 +80,21 @@ public class DirectoryDeletingService extends
AbstractKeyDeletingService {
// Use only a single thread for DirDeletion. Multiple threads would read
// or write to same tables and can send deletion requests for same key
// multiple times.
Review Comment:
Updated the comment.
--
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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
sumitagrawl commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1822437886
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##
@@ -257,8 +259,13 @@ public void start(OzoneConfiguration configuration) {
OZONE_BLOCK_DELETING_SERVICE_TIMEOUT,
OZONE_BLOCK_DELETING_SERVICE_TIMEOUT_DEFAULT,
TimeUnit.MILLISECONDS);
- dirDeletingService = new DirectoryDeletingService(dirDeleteInterval,
- TimeUnit.MILLISECONDS, serviceTimeout, ozoneManager, configuration);
+ int dirDeletingServiceCorePoolSize =
+ configuration.getInt(OZONE_THREAD_NUMBER_DIR_DELETION,
+ OZONE_THREAD_NUMBER_DIR_DELETION_DEFAULT);
+ dirDeletingService =
Review Comment:
can define min number of threads to avoid 0 or other value.
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -135,17 +143,66 @@ public void resume() {
@Override
public BackgroundTaskQueue getTasks() {
BackgroundTaskQueue queue = new BackgroundTaskQueue();
-queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+BlockingQueue> dirQueue =
+new ArrayBlockingQueue<>(1);
+// Iterate the deleted dir table and push the dir omKeyInfo in an Array
list.
+// Using a map to maintain unique elements in the list.
+int count = 0;
+final int MAX_COUNT = 1;
+try (
+TableIterator>
deleteTableIterator = getOzoneManager().getMetadataManager()
Review Comment:
we can pass suplier giving next parent on need basis, this will avoid
unnescessary polling of records getting discarded if not consumed.
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -135,17 +143,66 @@ public void resume() {
@Override
public BackgroundTaskQueue getTasks() {
BackgroundTaskQueue queue = new BackgroundTaskQueue();
-queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+BlockingQueue> dirQueue =
+new ArrayBlockingQueue<>(1);
+// Iterate the deleted dir table and push the dir omKeyInfo in an Array
list.
+// Using a map to maintain unique elements in the list.
+int count = 0;
+final int MAX_COUNT = 1;
+try (
+TableIterator>
deleteTableIterator = getOzoneManager().getMetadataManager()
+.getDeletedDirTable().iterator()) {
+ Table.KeyValue deletedDirInfo;
+ while (count < MAX_COUNT && deleteTableIterator.hasNext()) {
+deletedDirInfo = deleteTableIterator.next();
+if (uniqueIDs.add(deletedDirInfo.getValue().getObjectID())) {
+ dirQueue.add(deletedDirInfo);
+ count++;
+}
+ }
+} catch (IOException e) {
+ LOG.error(
+ "Error while running delete directories and files " + "background
task. Will retry at next run.",
+ e);
+}
+int numThreads = dirQueue.size() < DIR_DELETING_CORE_POOL_SIZE ? 1 :
+DIR_DELETING_CORE_POOL_SIZE;
+for (int i = 0; i < numThreads; i++) {
+ queue.add(new DirectoryDeletingService.DirDeletingTask(this, dirQueue));
+}
+cleanUniqueIdSet(uniqueIDs);
return queue;
}
+ private void cleanUniqueIdSet(Set uniqueIDs) {
+int count = 0;
+if (uniqueIDs.size() > 4) {
+ Iterator iterator = uniqueIDs.iterator();
+ while (iterator.hasNext()) {
+iterator.next();
+iterator.remove();
+count++;
+if (count >= 1)
+ break;
+ }
+}
+ }
+
private final class DirDeletingTask implements BackgroundTask {
private final DirectoryDeletingService directoryDeletingService;
+BlockingQueue> dirQueue =
Review Comment:
This must be initialized in constructor always as input
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -135,10 +142,65 @@ public void resume() {
@Override
public BackgroundTaskQueue getTasks() {
BackgroundTaskQueue queue = new BackgroundTaskQueue();
-queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+supplier.reInitItr();
+for (int i = 0; i < DIR_DELETING_CORE_POOL_SIZE; i++) {
+ queue.add(new DirectoryDeletingService.DirDeletingTask(this));
+}
return queue;
}
+ @Override
+ public void shutdown() {
+super.shutdown();
+supplier.closeItr();
+ }
+
+ private final class Supplier {
+TableIterator>
+deleteTableIterator;
+
+private Supplier() throws IOException {
+ this.deleteTableIterator =
+ getOzoneManager().getMetadataManager().getDeletedDirTable()
+ .iterator();
+}
+
+private synchronized Table.KeyValue getItr() {
+ if (deleteTableIterator != null && !deleteTableIterator.hasNext()) {
+try {
+ if (ge
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
errose28 commented on PR #7349: URL: https://github.com/apache/ozone/pull/7349#issuecomment-2445392341 Can you fill in the PR description to describe the algorithm implemented to divide the work among n threads? -- 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
Re: [PR] HDDS-11605. Directory deletion service should support multiple threads [ozone]
adoroszlai commented on code in PR #7349:
URL: https://github.com/apache/ozone/pull/7349#discussion_r1812285807
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -77,20 +80,21 @@ public class DirectoryDeletingService extends
AbstractKeyDeletingService {
// Use only a single thread for DirDeletion. Multiple threads would read
// or write to same tables and can send deletion requests for same key
// multiple times.
Review Comment:
Comment is outdated.
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -77,20 +80,21 @@ public class DirectoryDeletingService extends
AbstractKeyDeletingService {
// Use only a single thread for DirDeletion. Multiple threads would read
// or write to same tables and can send deletion requests for same key
// multiple times.
- private static final int DIR_DELETING_CORE_POOL_SIZE = 1;
+ private static int DIR_DELETING_CORE_POOL_SIZE;
private static final int MIN_ERR_LIMIT_PER_TASK = 1000;
// Number of items(dirs/files) to be batched in an iteration.
private final long pathLimitPerTask;
private final int ratisByteLimit;
private final AtomicBoolean suspended;
private AtomicBoolean isRunningOnAOS;
+ private Set uniqueIDs = new LinkedHashSet<>();
Review Comment:
Can be `final`.
##
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##
@@ -77,20 +80,21 @@ public class DirectoryDeletingService extends
AbstractKeyDeletingService {
// Use only a single thread for DirDeletion. Multiple threads would read
// or write to same tables and can send deletion requests for same key
// multiple times.
- private static final int DIR_DELETING_CORE_POOL_SIZE = 1;
+ private static int DIR_DELETING_CORE_POOL_SIZE;
Review Comment:
Variable should not be `static`.
--
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
