abdullah alamoudi has posted comments on this change. Change subject: ASTERIXDB-1917: FLUSH_LSN for disk components is not correctly set ......................................................................
Patch Set 1: > > The patch seems nice to me. I'm not sure I entirely understand > the > > proposed alternative. If I understand the bug correctly, the > issue > > here basically is that triggerFlushRequest will eagerly set the > > LSN, which is incorrect because it can and will fail to flush > when > > a request is already pending. Regardless of whether we persist > the > > LSN in the component (which does make more sense to do), or > inside > > the callback, doesn't the same issue apply? > > Yes, I think even if we persist the LSN into the memory component, > the same issue still applies since it is still possible to override > the component LSN (just as this bug). > > Moreover, I think it's better to keep the IOOperationCallback as it > is, otherwise the flush/merge API would carry some extra > information (like LSN in this case) to make things work. I am not saying that the fix is incorrect. Here is what is happening. We have a memory component's metadata with the memory component. This metadata already has pairs of key value. when we flush, we move those key values to the disk component: See: LSMBTree.java line 359: flushingComponent.getMetadata().copy(component.getMetadata()); what about the LSN? it is not yet written to the disk component. after the flush call returns, we do: operation.getCallback().afterOperation(LSMOperationType.FLUSH, null, newComponent); If we put the lsn when we schedule the operation inside the scheduleFlush() call, then it will be written to the metadata of the disk component with the other key value pairs. if the lsn was set later in the callback, it wouldn't affect the value that goes to disk. -- To view, visit https://asterix-gerrit.ics.uci.edu/1771 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: If438e34f8f612458d81f618eea04c0c72c49a9fe Gerrit-PatchSet: 1 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Luo Chen <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Luo Chen <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: No
