abdullah alamoudi has submitted this change and it was merged. Change subject: [NO ISSUE][STO] Skip deleting unallocated memory component ......................................................................
[NO ISSUE][STO] Skip deleting unallocated memory component - user model changes: no - storage format changes: no - interface changes: no Details: - This change fixes the component delete logic - It first ensures that memory is allocated for the index - Then it checks whether the memory component is to be deleted - This is important since there might be cases where primary index has the memory allocated but not the secondary and without allocating secondary and deleting it, we could end up with memory components with different ids. Change-Id: I0c6c7968830f3c9241bd036c0a330be1400349b4 Reviewed-on: https://asterix-gerrit.ics.uci.edu/2668 Sonar-Qube: Jenkins <[email protected]> Reviewed-by: abdullah alamoudi <[email protected]> Integration-Tests: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> Contrib: Jenkins <[email protected]> Reviewed-by: Michael Blow <[email protected]> Reviewed-by: Murtadha Hubail <[email protected]> --- M asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/PrimaryIndexOperationTracker.java M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMHarness.java 2 files changed, 25 insertions(+), 17 deletions(-) Approvals: Anon. E. Moose #1000171: abdullah alamoudi: Looks good to me, but someone else must approve Jenkins: Verified; No violations found; ; Verified Michael Blow: Looks good to me, but someone else must approve Murtadha Hubail: Looks good to me, approved diff --git a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/PrimaryIndexOperationTracker.java b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/PrimaryIndexOperationTracker.java index 0aa0d2b..a1d31d5 100644 --- a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/PrimaryIndexOperationTracker.java +++ b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/PrimaryIndexOperationTracker.java @@ -132,7 +132,8 @@ for (ILSMIndex lsmIndex : indexes) { if (lsmIndex.isPrimaryIndex()) { if (lsmIndex.isCurrentMutableComponentEmpty()) { - LOGGER.info("Primary index on dataset {} and partition {} is empty... skipping flush"); + LOGGER.info("Primary index on dataset {} and partition {} is empty... skipping flush", + dsInfo.getDatasetID(), partition); return; } primaryLsmIndex = lsmIndex; diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMHarness.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMHarness.java index 7a87a13..4d840b0 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMHarness.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMHarness.java @@ -483,10 +483,10 @@ public ILSMIOOperation scheduleFlush(ILSMIndexOperationContext ctx) throws HyracksDataException { ILSMIOOperation flush; LOGGER.info("Flush is being scheduled on {}", lsmIndex); + if (!lsmIndex.isMemoryComponentsAllocated()) { + lsmIndex.allocateMemoryComponents(); + } synchronized (opTracker) { - if (!lsmIndex.isMemoryComponentsAllocated()) { - lsmIndex.allocateMemoryComponents(); - } try { flush = lsmIndex.createFlushOperation(ctx); } finally { @@ -738,32 +738,36 @@ @Override public void deleteComponents(ILSMIndexOperationContext ctx, Predicate<ILSMComponent> predicate) throws HyracksDataException { - boolean deleteMemoryComponent; ILSMIOOperation ioOperation = null; + // We need to always start the component delete from current memory component. + // This will ensure Primary and secondary component id still matches after component delete + if (!lsmIndex.isMemoryComponentsAllocated()) { + lsmIndex.allocateMemoryComponents(); + } synchronized (opTracker) { waitForFlushesAndMerges(); // We always start with the memory component ILSMMemoryComponent memComponent = lsmIndex.getCurrentMemoryComponent(); - deleteMemoryComponent = predicate.test(memComponent); - if (deleteMemoryComponent) { + if (predicate.test(memComponent)) { // schedule a delete for flushed component ctx.reset(); ctx.setOperation(IndexOperation.DELETE_COMPONENTS); ioOperation = scheduleFlush(ctx); + } else { + // since we're not deleting the memory component, we can't delete any previous component + return; } } // Here, we are releasing the opTracker to allow other operations: // (searches, delete flush we will schedule, delete merge we will schedule). - if (deleteMemoryComponent) { - try { - ioOperation.sync(); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw HyracksDataException.create(e); - } - if (ioOperation.getStatus() == LSMIOOperationStatus.FAILURE) { - throw HyracksDataException.create(ioOperation.getFailure()); - } + try { + ioOperation.sync(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw HyracksDataException.create(e); + } + if (ioOperation.getStatus() == LSMIOOperationStatus.FAILURE) { + throw HyracksDataException.create(ioOperation.getFailure()); } ctx.reset(); ctx.setOperation(IndexOperation.DELETE_COMPONENTS); @@ -774,6 +778,9 @@ for (ILSMDiskComponent component : diskComponents) { if (predicate.test(component)) { ctx.getComponentsToBeMerged().add(component); + } else { + // Can't delete older components when newer one is still there + break; } } if (ctx.getComponentsToBeMerged().isEmpty()) { -- To view, visit https://asterix-gerrit.ics.uci.edu/2668 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: I0c6c7968830f3c9241bd036c0a330be1400349b4 Gerrit-PatchSet: 7 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]>
