tkalkirill commented on code in PR #2126:
URL: https://github.com/apache/ignite-3/pull/2126#discussion_r1214047493
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/gc/GcUpdateHandler.java:
##########
@@ -65,56 +65,59 @@ public PendingComparableValuesTracker<HybridTimestamp,
Void> getSafeTimeTracker(
/**
* Tries removing {@code count} oldest stale entries and their indexes.
- * If there's less entries that can be removed, then exits prematurely.
+ * If there are fewer rows than the {@code count}, or it was not possible
to lock any, then exits prematurely.
*
* @param lowWatermark Low watermark for the vacuum.
* @param count Count of entries to GC.
+ * @return {@code False} if there is no garbage left in the storage.
*/
- public void vacuumBatch(HybridTimestamp lowWatermark, int count) {
- for (int i = 0; i < count; i++) {
- if (!storage.runConsistently(locker ->
internalVacuum(lowWatermark))) {
- break;
+ public boolean vacuumBatch(HybridTimestamp lowWatermark, int count) {
+ return storage.runConsistently(locker -> {
+ for (int i = 0; i < count; i++) {
+ if (!internalVacuum(lowWatermark, locker)) {
+ return false;
+ }
}
- }
- }
- /**
- * Tries removing partition's oldest stale entry and its indexes.
- *
- * @param lowWatermark Low watermark for the vacuum.
- * @return {@code true} if an entry was garbage collected, {@code false}
if there was nothing to collect.
- * @see MvPartitionStorage#pollForVacuum(HybridTimestamp)
- */
- public boolean vacuum(HybridTimestamp lowWatermark) {
- return storage.runConsistently(locker -> internalVacuum(lowWatermark));
+ return true;
+ });
}
/**
- * Executes garbage collection.
+ * Attempts to collect garbage for one {@link RowId}, if it fails to lock
it, then immediately breaks.
*
- * <p>Must be called inside a {@link
MvPartitionStorage#runConsistently(WriteClosure)} closure.
+ * <p>Must be called inside a {@link
PartitionDataStorage#runConsistently(WriteClosure)} closure.
*
* @param lowWatermark Low watermark for the vacuum.
- * @return {@code true} if an entry was garbage collected, {@code false}
if there was nothing to collect.
+ * @param locker From {@link
PartitionDataStorage#runConsistently(WriteClosure)}.
+ * @return {@code False} if there is no garbage left in the {@link
#storage}.
*/
- private boolean internalVacuum(HybridTimestamp lowWatermark) {
- BinaryRowAndRowId vacuumed = storage.pollForVacuum(lowWatermark);
+ private boolean internalVacuum(HybridTimestamp lowWatermark, Locker
locker) {
+ while (true) {
+ GcEntry gcEntry = storage.peek(lowWatermark);
- if (vacuumed == null) {
- // Nothing was garbage collected.
- return false;
- }
+ if (gcEntry == null) {
+ return false;
+ }
- BinaryRow binaryRow = vacuumed.binaryRow();
+ RowId rowId = gcEntry.getRowId();
- assert binaryRow != null;
+ if (!locker.tryLock(rowId)) {
Review Comment:
What to prevent possible deadlocks within a single **runConsistently**.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
