asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Murtadha Hubail (Code Review)" <>
Subject Change in asterixdb[master]: ASTERIXDB-969: Redesigned recovery analysis phase to spill t...
Date Thu, 29 Oct 2015 02:02:13 GMT
Murtadha Hubail has posted comments on this change.

Change subject: ASTERIXDB-969: Redesigned recovery analysis phase to spill to disk

Patch Set 1:

File asterix-transactions/src/main/java/org/apache/asterix/transaction/management/service/recovery/

Line 213:                         if (jobId2WinnerEntitiesMap.containsKey(jobId)) {
> Here, jobId is not updated.

Line 217:                             jobId2WinnerEntitiesMap.remove(logRecord.getJobId());
> let's be consistent: either use jobid or logRecord.getJobId().

Line 225:                             if (needToFreeMemroy()) {
> Typo: needToFreeMemory

Line 574:         if (!jobRecoveryFile.exists()) {
> How can the file exist? Should this be an exception?
I changed it to be an exception if it already exists.

Line 834:             if (preparedForSearch) {
> This method is called only once per winnerEntitySet. So, this check seems u
This is just a protection in case someone calls it twice by mistake. I removed it anyway.

Line 909:             if (needToFreeMemroy()) {
> Are these check and free operation necessary? It seems that the same operat
The next step after this line allocates a buffer that can hold this in memory partition. If
there isn't enough memory, it will throw an error. That's why this check is needed. I can
eliminate it by writing the in-memory cached entities to the file channel one by one, but
I don't think it is worth it.

Line 921:             File partitionFile = createJobRecoveryFile(jobId, getPartitonName(partitionMaxLSN));
> typo: getPartitionName

Line 923:             try (FileOutputStream fileOutputstream = new FileOutputStream(partitionFile,
> what is this try for?
This is try with resources that automatically closes the file channel at the end of the try

Line 926:                 fileChannel.write(buffer);
> busy waiting and write everything

Line 927:                 fileChannel.force(true);
> force seems not necessary. If there is a crash, anyway everything will be d

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide2b346c2ad498d7595e71bae890362c2143d301
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <>
Gerrit-Reviewer: Jenkins <>
Gerrit-Reviewer: Murtadha Hubail <>
Gerrit-Reviewer: Young-Seok Kim <>
Gerrit-HasComments: Yes

View raw message