Ian Maxon has posted comments on this change.

Change subject: ASTERIXDB-1045: Log analysis fixes
......................................................................


Patch Set 14:

(4 comments)

https://asterix-gerrit.ics.uci.edu/#/c/289/14/asterix-common/src/main/java/org/apache/asterix/common/transactions/LogRecord.java
File 
asterix-common/src/main/java/org/apache/asterix/common/transactions/LogRecord.java:

Line 160:         if(buffer.remaining() < (Integer.SIZE + Byte.SIZE)/Byte.SIZE) 
{
> better be defining constants such as LOG_TYPE_SIZE, JOB_ID_SIZE. Also, belo
I had thought about this, but I wasn't sure since we just do readInt()/etc. 
I'll change it now though.


https://asterix-gerrit.ics.uci.edu/#/c/289/14/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/service/logging/LogReader.java
File 
asterix-transactions/src/main/java/org/apache/asterix/transaction/management/service/logging/LogReader.java:

Line 105:                 }
> what if the record's status is BAD_CHKSUM? Also, below, we should distingui
No, we shouldn't ever throw a runtime exception here. All that should happen is 
that recovery stops earlier than normal. You're right about the inner if()  
though, on the second try the message should distinguish between truncation and 
bad checksums.


Line 172:                 read = fileChannel.read(readBuffer);
> Can we explain briefly the read() method behavior so that code reader under
Certainly.


Line 220:             eof = refillLogReadBuffer();
> should be 
Done


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/289
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1658e938eb0f199f748407361ffee4833aac661c
Gerrit-PatchSet: 14
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <[email protected]>
Gerrit-Reviewer: Ian Maxon <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Murtadha Hubail <[email protected]>
Gerrit-Reviewer: Young-Seok Kim <[email protected]>
Gerrit-HasComments: Yes

Reply via email to