Luo Chen 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.

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

Reply via email to