You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "ivandika3 (via GitHub)" <gi...@apache.org> on 2026/07/27 02:12:05 UTC

[PR] HDDS-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

ivandika3 opened a new pull request, #10876:
URL: https://github.com/apache/ozone/pull/10876

   ## What changes were proposed in this pull request?
   
   We should remove setExclusiveManualCompaction so that automatic compactions can still run to compact L0 -> L1 files and prevent write stalls.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-15990
   
   ## How was this patch tested?
   
   CI.


-- 
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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5131339722

   Thanks @ashishkumar50 @jojochuang @ptlrs @swamirishi for the reviews.


-- 
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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5101404810

   > > > Is there some bulk delete in the buckets?
   > > 
   > > 
   > > @swamirishi Yes, we have the S3 lifecycle configuration service ([HDDS-8342](https://issues.apache.org/jira/browse/HDDS-8342)) that generates a few millions deletions in a very short time. In LEGACY bucket `getFileStatus` can trigger a RocksDB seek that is done under a bucket lock, if there are a lot of tombstones, then the lock will be held for a long time which can block Ratis applier and cause the OM write to be stuck. We have resolved it in #9932 by moving the createFakeDirIfShould outside the lock, but new patches can easily fall to the same trap since some people might thing that RocksDB seek behaves like a point query rather than a range query.
   > > > We can do something similar for the delete as well and add delete range tombstone as well.
   > > 
   > > 
   > > The lifecycle configuration service cannot use deleteRange (unlike DirectoryDeletingService since AFAIK DirectoryDeletingService only handles orphan directories and no keys will be created under the directory anymore) since another key can be created in the deleteRange which can cause valid keys to be inadvertently deleted, causing data loss.
   > 
   > No that should not happen as long as you decide to do the iteration and delete range inside validateAndUpdateCache method of
   > 
   > https://github.com/apache/ozone/blob/daef2f19c2946ad8e909323f5956b1884bba64fe/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java#L117
   > 
   > 
   > We can sort the keys to be deleted and initialize a rocksdb iterator for key table and do delete range for contiguous keys to be deleted in the iterator. Basically have a 2 pointer iteration b/w the keys to be deleted and the rocksdb iterator this would figure out the contiguous keys to be deleted. If the million of keys are contiguous it would optimize the number of tombstones we create by a huge magnitude. Basically I would propose to change this unoptimal loop which does individual seeks and get for each delete key which can be very unoptimal in every case having a rocksdb iterator would be way more optimal.
   > https://github.com/apache/ozone/blob/daef2f19c2946ad8e909323f5956b1884bba64fe/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java#L187-L192
   > 
   > 
   > I am assuming you guys are using the bulk delete api to delete since I don't think it would be possible to do millions of deletes when deleting individual keys anyhow because of our om ratis constraints.
   > > > Do you guys have ACL bloat problem?
   > > 
   > > 
   > > We previously had one, but we already resolved it so that now a single key usually only have 1 ACL only. Our custom authorizer allows us to have a stable ACL per key.
   > > But each OmKeyInfo can be large due to a lot of blocks (for example, if a key is uploaded using MPU with very small parts), so the risk is still there unless we make OmKeyInfo to not contain any unbounded list (i.e. ACLs and Blocks).
   
   I see you already have a ticket for this https://issues.apache.org/jira/browse/HDDS-14613 . Let us work on this particular change.


-- 
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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5127311466

   > When you do seek it doesn't iterate through all the tombstones it would try to search in the LSM using indexes in the memtable and ssts to efficiently figure out the keys. Only iterator.next() would have troubles when there are lot of tombstones. With deleteRange we are actually solving the tombstone problem and thus solving the slow iterator problem. With deleteRange the iterator would skip an entire range instead of having to go through each tombstone on by one.
   
   <img width="1404" height="1298" alt="image" src="https://github.com/user-attachments/assets/48293f7a-9b18-4f4a-90b4-75ae3d612df5" />
   


-- 
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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "ptlrs (via GitHub)" <gi...@apache.org>.
ptlrs commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5111887377

   Thanks for the PR @ivandika3. Removing this configuration should be ok. As per the RocksDB source, `exclusive_manual_compaction=true` is not required since 2021/prior to 7.7.3. 
   
   ```c
     // When a manual compaction arrives, temporarily disable scheduling of
     // non-manual compactions and wait until the number of scheduled compaction
     // jobs drops to zero. This used to be needed to ensure that this manual
     // compaction can compact any range of keys/files. Now it is optional
     // (see `CompactRangeOptions::exclusive_manual_compaction`). The use case for
     // `exclusive_manual_compaction=true` is unclear beyond not trusting the code.
   ```
   - https://github.com/facebook/rocksdb/blob/01cd296b079d1c82e28bfddd6850d7de876aaf6d/db/db_impl/db_impl_compaction_flush.cc#L1858-L1864
   - https://github.com/facebook/rocksdb/pull/10317
   
   This scenario has essentially stalled writes by creating a backpressure in the write flow. While removing this configuration helps, the scenario may still reproduce if L0 to L1 compaction slows down due to any other reason. We may need to revisit the values for max_subcompactions, max_background_compactions and max_background_jobs. 


-- 
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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5125918601

   > As per the RocksDB source, exclusive_manual_compaction=true is not required since 2021/prior to 7.7.3.
   
   Yes this means that there is no point of adding `exclusive_manual_compaction` and we just added this arbitrarily.


-- 
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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5128676308

   > KforceOptimized potentially reduces writeAmplifications by ensuring the files which were newly generated in the compaction doesn't get redundantly get compacted again.
   
   Ok, sounds valid.
   
   >  Let us go through an example of an iterator.
   
   Thanks for the explanation. I see the benefit of deleteRange if most a lot of large ranges are covered by deleteRange.
   
   > With the proposed changes we would not have a lot of tombstones to begin with and only multiple range tombstones which would not be in big numbers thus the iterator overhead would be minimal not as major as you would think
   
   This might not be assumed, if user sends a lot of delete requests, tombstones might still be a lot. Also for existing cluster which high tombstones, the overhead still happens.
   
   @swamirishi Btw, please help to approve this if you think it looks good so I can merge 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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5102785094

   > > The first key in iterator is k00 and delete keys is k01. If pointer at deleted key is greater than iterator then iterator seeks to deleted key by adding the current deleteRange to the deleteRangeList. So the iterator is at k01 now.
   > 
   > Let's say that between k00 and k01 as well as k02 and k04, there are 1 million tombstones, the RocksDB iterator is forced to iterate all these 1 million tombstone entries. This will make performance unpredictable.
   > 
   > The deleteRange on [k01, k03) is not going to be worth it in that case.
   > 
   > There is a RocksDB multiget https://github.com/facebook/rocksdb/wiki/MultiGet-Performance to improve performance of multiget, we can probably explore this, but I don't think the proposed algorithm above is the way to go.
   > 
   > The rule of thumb for me is to never use range query (even if it's implicit like RocksDB seek) if you are planning to do point query / queries.
   
   Seek should be a relatively safe. But I just realized we cannot use deleteRange because of snapshots since snapshot diff depends on the individual tombstone entry for deletes.
   
   > 
   > @swamirishi Anyway, we've digressed from the purpose of the original implementation, let's continue the discussion in [HDDS-14613](https://issues.apache.org/jira/browse/HDDS-14613)
   
   Yeah sure. As regards to this change I am fine with the change. I believe we should change the Compaction type to kForceOptimized.


-- 
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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5087700779

   Thanks @ashishkumar50 for checking this patch.
   
   > Thanks for the patch, whether you are using kSkip or kForce for the compaction?
   As in below PR now compaction is using kSkip by default which will not compact bottommost layer and should be fast enough.
   
   We are using `kForce`, https://github.com/apache/ozone/pull/10419 is a very recent patch so we are not using this.
   
   However, I don't see any good reason why we have to `setExclusiveManualCompaction` rather than just prematurely preventing hypothetical overhead of concurrent compactions (which I don't think is well understood by the Ozone community). For example, we already have a previous `DatanodeStoreSchemaThreeImpl#compactionIfNeeded` in datanode which does not `setExclusiveManualCompaction` and we have no write stall issues. So unless there is a stress test proving that the concurrent manual and automatic compaction actually has any meaningful overhead, we should remove it since now we know that there is a very real production risk (not just hypotheticals).
   
   From my understanding the `kForce` is the entire point of the compaction service. If there are a lot of tombstones in the bottommost layer, we want to clean them up. Additionally, is the difference between kSkip and automatic compaction well-understood?
   
   > should be fast enough
   
   If the N-1 LSM level is very big, the issue might still happen. Unless we have a strong guarantee or data of no write stall, IMO this exclusive compaction adds another unnecessary surprise in OM stability.


-- 
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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5102405726

   > The first key in iterator is k00 and delete keys is k01. If pointer at deleted key is greater than iterator then iterator seeks to deleted key by adding the current deleteRange to the deleteRangeList. So the iterator is at k01 now.
   
   Let's say that between k00 and k01, there are 1 millions tombstones, the RocksDB iterator is forced to iterate all these 100,000 entries. This will make performance unpredictable.
   
   The deleteRange on [k01, k03) is not going to be worth it in that case. 
   
   There is a RocksDB multiget https://github.com/facebook/rocksdb/wiki/MultiGet-Performance to improve performance of multiget, we can probably explore this, but I don't think the proposed algorithm above is the way to go.
   


-- 
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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5091889852

   > However, I don't see any good reason why we have to setExclusiveManualCompaction rather than just prematurely preventing hypothetical overhead of concurrent compactions (which I don't think is well understood by the Ozone community). For example, we already have a previous DatanodeStoreSchemaThreeImpl#compactionIfNeeded in datanode which does not setExclusiveManualCompaction and we have no (known) write stall issues (we still need to check this). So unless there is a stress test proving that the concurrent manual and automatic compaction actually has any meaningful overhead, we should remove it since now we know that there is a very real production risk (not just hypotheticals).
   
   @jojochuang Do you have any thought on this, why setExclusiveManualCompaction was done?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5100287357

   > > Probably a good idea to revisit the built-in mechanisms rather than reinvent the wheels.
   > 
   > +1, need to revisit RocksDB as a whole.
   > 
   > > this is great writeup. Has this been tested in your prod environment? If so let's merge it asap.
   > 
   > Not yet since we need to reproduce it in our test cluster. We have previously run some stress test in our test cluster to check the risk of the compaction service, but the issue is not triggered during the test so not sure how hard it is to reproduce it in our test cluster.
   > 
   > Currently, we decided to disable compaction service for the foreseeable future until we understand the RocksDB enough to be confident to enable it, the risk is too high.
   > 
   > > We won't have the tombstone and slow rocksdb iterator problem to begin with.
   > 
   > @swamirishi Seems your patches are related to deleteRange and snapshot. Our cluster does encounter tombstone and slow rocksdb iterator caused by holding the RocksDB iterator during RocksDB seek in the LEGACY bucket , see #9932 . Not sure how the attached patches will help prevent our case (we don't use Ozone snapshot in production).
   
   I am sorry @ivandika3 I redundantly added links to the same patches twice and forgot to add this. The following [PR](https://github.com/apache/ozone/pull/9423) solves slow iterator problem for FSO. For OBS I would love to understand how the keys are being deleted in production. Is there some bulk delete in the buckets? We can do something similar for the delete as well and add delete range tombstone as well. BTW can you check what is the size of each OMKeyInfo in your keyTable/FileTable/DirectoryTable. Do you guys have ACL bloat problem?
   


-- 
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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 merged PR #10876:
URL: https://github.com/apache/ozone/pull/10876


-- 
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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5125945661

   >  believe we should change the Compaction type to kForceOptimized.
   
   @swamirishi please provide the full reasoning why kForceOptimized is better.


-- 
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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5101357018

   > > > Is there some bulk delete in the buckets?
   > > 
   > > 
   > > @swamirishi Yes, we have the S3 lifecycle configuration service ([HDDS-8342](https://issues.apache.org/jira/browse/HDDS-8342)) that generates a few millions deletions in a very short time. In LEGACY bucket `getFileStatus` can trigger a RocksDB seek that is done under a bucket lock, if there are a lot of tombstones, then the lock will be held for a long time which can block Ratis applier and cause the OM write to be stuck. We have resolved it in #9932 by moving the createFakeDirIfShould outside the lock, but new patches can easily fall to the same trap since some people might thing that RocksDB seek behaves like a point query rather than a range query.
   > > > We can do something similar for the delete as well and add delete range tombstone as well.
   > > 
   > > 
   > > The lifecycle configuration service cannot use deleteRange (unlike DirectoryDeletingService since AFAIK DirectoryDeletingService only handles orphan directories and no keys will be created under the directory anymore) since another key can be created in the deleteRange which can cause valid keys to be inadvertently deleted, causing data loss.
   > 
   > No that should not happen as long as you decide to do the iteration and delete range inside validateAndUpdateCache method of
   > 
   > https://github.com/apache/ozone/blob/daef2f19c2946ad8e909323f5956b1884bba64fe/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java#L117
   > 
   > 
   > We can sort the keys to be deleted and initialize a rocksdb iterator for key table and do delete range for contiguous keys to be deleted in the iterator. Basically have a 2 pointer iteration b/w the keys to be deleted and the rocksdb iterator this would figure out the contiguous keys to be deleted. If the million of keys are contiguous it would optimize the number of tombstones we create by a huge magnitude. Basically I would propose to change this unoptimal loop which does individual seeks and get for each delete key which can be very unoptimal in every case having a rocksdb iterator would be way more optimal.
   > https://github.com/apache/ozone/blob/daef2f19c2946ad8e909323f5956b1884bba64fe/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java#L187-L192
   
   I am assuming you guys are using the bulk delete api to delete since I don't think it would be possible to do millions of deletes when deleting individual keys anyhow because of our om ratis constraints.
   
   > 
   > > > Do you guys have ACL bloat problem?
   > > 
   > > 
   > > We previously had one, but we already resolved it so that now a single key usually only have 1 ACL only. Our custom authorizer allows us to have a stable ACL per key.
   > > But each OmKeyInfo can be large due to a lot of blocks (for example, if a key is uploaded using MPU with very small parts), so the risk is still there unless we make OmKeyInfo to not contain any unbounded list (i.e. ACLs and Blocks).
   
   


-- 
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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5100606685

   > Is there some bulk delete in the buckets? We can do something similar for the delete as well and add delete range tombstone as well.
   
   @swamirishi Yes, we have the S3 lifecycle configuration service (HDDS-8342) that generates a few millions deletions in a very short time. In LEGACY bucket `getFileStatus` can trigger a RocksDB seek that is done under a bucket lock, if there are a lot of tombstones, then the lock will be held for a long time which can block Ratis applier and cause the OM write to be stuck. The lifecycle configuration service cannot use deleteRange (unlike DirectoryDeletingService) since another key can be created in the deleteRange which can cause valid keys to be inadvertently deleted.
   
   > Do you guys have ACL bloat problem?
   
   We previously had one, but we already resolved it so that now a single key usually only have 1 ACL only. Our custom authorizer allows us to have a stable ACL. 
   
   But each OmKeyInfo can be large due to a lot of blocks (for example, if a key is uploaded using MPU with very small parts), so the risk is still there.


-- 
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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5098778299

   > Probably a good idea to revisit the built-in mechanisms rather than reinvent the wheels.
   
   +1
   
   > this is great writeup. Has this been tested in your prod environment? If so let's merge it asap.
   
   Not yet. We decided to disable compaction service for the foreseeable future until we understand the RocksDB enough to be confident to enable 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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5101419759

   > No that should not happen as long as you decide to do the iteration and delete range inside validateAndUpdateCache method of
   
   This will kill performance, we cannot iterate every single key in validateAndUpdateCache and split them into multiple deleteRange groups since it will block subsequent writes. The lifecycle configuration scanning should not be run in a critical path.


-- 
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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5127328509

   > RocksDB source, `exclusive_manual_compaction=true` is not required since 2021/prior to 7.7.3.
   
   
   
   > > believe we should change the Compaction type to kForceOptimized.
   > 
   > @swamirishi please provide the full reasoning why kForceOptimized is better.
   
   KforceOptimized potentially reduces writeAmplifications by ensuring the files which were newly generated in the compaction doesn't get redundantly get compacted again.
   
   > 
   > > Seek should be a relatively safe
   > 
   > Also please provide reasoning how this is "relatively safe", relative to what? Unless seek is a point query and will not need to iterate over tombstones, seek is unsafe.
   > 
   > > But I just realized we cannot use deleteRange because of snapshots since snapshot diff depends on the individual tombstone entry for deletes.
   > 
   > Then all the deleteRange patch to OM need to be prohibited until snapshot feature handles it.
   > 
   > Regarding the multi-get idea, I don't think it's possible since OM still uses the table cache and RocksDB cannot update both the table cache (which is Ozone logic) and the RocksDB itself. In that case, we can consider whether this OM table cache (double buffer, etc) is actually worth the effort or just another premature optimization (we can check how other Raft + RocksDB system like TiKV actually does it). We can then consider removing OM table cache entirely. Table cache is also a source of inconsistency in OM since linearizability seem to be violated (see #10748) and I don't think anyone (including me) actually knows the consistency guarantee to the table cache since the design document never specifies 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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5127244469

   > Then all the deleteRange patch to OM need to be prohibited until snapshot feature handles it.
   
   
   
   > > believe we should change the Compaction type to kForceOptimized.
   > 
   > @swamirishi please provide the full reasoning why kForceOptimized is better.
   > 
   > > Seek should be a relatively safe
   
   > 
   > Also please provide reasoning how this is "relatively safe", relative to what? Unless seek is a point query and will not need to iterate over tombstones, seek is unsafe.
   
   When you do seek it doesn't iterate through all the tombstones it would try to search in the LSM using indexes in the memtable and ssts to efficiently figure out the keys. Only iterator.next() would have troubles when there are lot of tombstones. With deleteRange we are actually solving the tombstone problem and thus solving the slow iterator problem. With deleteRange the iterator would skip an entire range instead of having to go through each tombstone on by one.  
   
   > 
   > > But I just realized we cannot use deleteRange because of snapshots since snapshot diff depends on the individual tombstone entry for deletes.
   > 
   > Then all the deleteRange patch to OM need to be prohibited until snapshot feature handles it.
   > 
   
   We cannot support DeleteRange on the user path on keyTable/DirTable/FileTable(background services are ok) till the point non defragmented snapshot diff doesn't understand deleteRange tombstones. If we add a simple loop to identify the keys with deleteRange on the snapshot diff flows we can find the diff then. 
   
   > Regarding the multi-get idea, I don't think it's possible since OM still uses the table cache and RocksDB cannot update both the table cache (which is Ozone logic) and the RocksDB itself. In that case, we can consider whether this OM table cache (double buffer, etc) is actually worth the effort or just another premature optimization (we can check how other Raft + RocksDB system like TiKV actually does it). We can then consider removing OM table cache entirely. Table cache is also a source of inconsistency in OM since linearizability seem to be violated (see #10748) and I don't think anyone (including me) actually knows the consistency guarantee to the table cache since the design document never specifies 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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5095673860

   I don't really recall. Though I'd suggest to keep it simple rather than some kind of heuristics to predict whether to add this flag or not. With the recent RocksDB update (from 7 -> 10: #9813) a number of built-in compaction kicks in automatically. Probably a good idea to revisit the built-in mechanisms rather than reinvent the wheels.
   
   E.g. https://issues.apache.org/jira/browse/HDDS-12254


-- 
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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5095934688

   @ivandika3 this is great writeup. Has this been tested in your prod environment? If so let's merge it asap.


-- 
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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5096440913

   IMO we don't have the need of having CompactionService in OM. Om rocksdb has problems with the way we are using rocksdb. On a long term basis I would suggest we actually merge these prs and get rid of background compaction service altogether and let rocksdb take care of compactions:
   https://github.com/apache/ozone/pull/9560
   https://github.com/apache/ozone/pull/9553
   We won't have the tombstone and slow rocksdb iterator problem to begin with.


-- 
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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5101333840

   > > Is there some bulk delete in the buckets?
   > 
   > @swamirishi Yes, we have the S3 lifecycle configuration service ([HDDS-8342](https://issues.apache.org/jira/browse/HDDS-8342)) that generates a few millions deletions in a very short time. In LEGACY bucket `getFileStatus` can trigger a RocksDB seek that is done under a bucket lock, if there are a lot of tombstones, then the lock will be held for a long time which can block Ratis applier and cause the OM write to be stuck. We have resolved it in #9932 by moving the createFakeDirIfShould outside the lock, but new patches can easily fall to the same trap since some people might thing that RocksDB seek behaves like a point query rather than a range query.
   > 
   > > We can do something similar for the delete as well and add delete range tombstone as well.
   > 
   > The lifecycle configuration service cannot use deleteRange (unlike DirectoryDeletingService since AFAIK DirectoryDeletingService only handles orphan directories and no keys will be created under the directory anymore) since another key can be created in the deleteRange which can cause valid keys to be inadvertently deleted, causing data loss.
   
   No that should not happen as long as you decide to do the iteration and delete range inside validateAndUpdateCache method of https://github.com/apache/ozone/blob/daef2f19c2946ad8e909323f5956b1884bba64fe/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java#L117
   We can sort the keys to be deleted and initialize a rocksdb iterator for key table and do delete range for contiguous keys to be deleted in the iterator. Basically have a 2 pointer iteration b/w the keys to be deleted and the rocksdb iterator this would figure out the contiguous keys to be deleted. If the million of keys are contiguous it would optimize the number of tombstones we create by a huge magnitude. Basically I would propose to change this unoptimal loop which does individual seeks and get for each delete key which can be very unoptimal in every case having a rocksdb iterator would be way more optimal.
   https://github.com/apache/ozone/blob/daef2f19c2946ad8e909323f5956b1884bba64fe/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java#L187-L192
   
   > 
   > > Do you guys have ACL bloat problem?
   > 
   > We previously had one, but we already resolved it so that now a single key usually only have 1 ACL only. Our custom authorizer allows us to have a stable ACL per key.
   > 
   > But each OmKeyInfo can be large due to a lot of blocks (for example, if a key is uploaded using MPU with very small parts), so the risk is still there unless we make OmKeyInfo to not contain any unbounded list (i.e. ACLs and Blocks).
   
   


-- 
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-15990. Remove setExclusiveManualCompaction from CompactionService [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #10876:
URL: https://github.com/apache/ozone/pull/10876#issuecomment-5101524609

   > > No that should not happen as long as you decide to do the iteration and delete range inside validateAndUpdateCache method of
   > 
   > @swamirishi This will kill performance, we cannot iterate every single key in validateAndUpdateCache and split them into multiple deleteRange groups since it will block subsequent writes. The lifecycle configuration scanning should not be run in a critical path.
   > 
   > > I see you already have a ticket for this https://issues.apache.org/jira/browse/HDDS-14613 . Let us work on this particular change.
   > 
   > I closed that for the reason above.
   
   Not exactly IMO the iteration that we are doing is very unoptimal. The actual algo would be something like this:
   Say we want to delete 
   k01, k02, k04, k05, k10, k11
   and the key table has keys from k00..k99
   
   1) Init rocksdb iterator. Create a tmpList to keep track of OmKeyInfos and deleteRangeList
   2) The first key in iterator is k00 and delete keys is k01. If pointer at deleted key is greater than iterator then iterator seeks to deleted key by adding the current deleteRange to the  deleteRangeList. So the iterator is at k01 now.
   3) Both iterator is k01 and delete key is k01 so add k01 to OmKeyInfoList and move pointer of delete Key list to k02 and itr.next(). Similarly K02 will match and would be added as well and we proceed to next.
   4) Now DeleteKey is at K04 and rocksdbIterator is at k03. So we add deleteKeyRange [k01, k03) in deleteKeyRangeList and seek to k04. 
   The algo proceeds on like this till we don't iterate through all keys in the deleteKeyList. This would be much more optimal than whatever we are doing right now.
   Every rocksdb get is going to be an equivalent of randomized get in the LSM which would be very unoptimal to sequential gets.


-- 
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