asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Young-Seok Kim (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
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:

(10 comments)

Please address comments.

https://asterix-gerrit.ics.uci.edu/#/c/458/1/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/service/recovery/RecoveryManager.java
File asterix-transactions/src/main/java/org/apache/asterix/transaction/management/service/recovery/RecoveryManager.java:

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,
false);
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 https://asterix-gerrit.ics.uci.edu/458
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide2b346c2ad498d7595e71bae890362c2143d301
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hubailmor@gmail.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Young-Seok Kim <kisskys@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message