asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Young-Seok Kim (Code Review)" <>
Subject Change in asterixdb[master]: ASTERIXDB-969: Redesigned recovery analysis phase to spill t...
Date Sun, 25 Oct 2015 05:18:20 GMT
Young-Seok Kim has posted comments on this change.

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

Patch Set 1:


Please address comments.
File asterix-transactions/src/main/java/org/apache/asterix/transaction/management/service/recovery/

Line 213:                         if (jobId2WinnerEntitiesMap.containsKey(jobId)) {
Here, jobId is not updated.
jobId should be set before it's used.

Line 217:                             jobId2WinnerEntitiesMap.remove(logRecord.getJobId());
let's be consistent: either use jobid or logRecord.getJobId().
Seems jobId better than calling logRecord.getJobId() repeatedly

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

Line 574:         if (!jobRecoveryFile.exists()) {
How can the file exist? Should this be an exception?

Line 834:             if (preparedForSearch) {
This method is called only once per winnerEntitySet. So, this check seems useless.?

Line 909:             if (needToFreeMemroy()) {
Are these check and free operation necessary? It seems that the same operations in analysis
phase are sufficient?

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

Line 923:             try (FileOutputStream fileOutputstream = new FileOutputStream(partitionFile,
what is this try for?

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 deleted and start
over, right?

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: Young-Seok Kim <>
Gerrit-HasComments: Yes

View raw message