asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Blow (Code Review)" <>
Subject Change in asterixdb[master]: Add Test NodeController, Test Data Generator, and Marker Logs
Date Tue, 19 Jul 2016 04:02:56 GMT
Michael Blow has posted comments on this change.

Change subject: Add Test NodeController, Test Data Generator, and Marker Logs

Patch Set 10:

File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/

Line 733:                             : storageProperties.getMemoryComponentNumPages();
not a fan, i think the original version is much easier to read

Line 759:             int numPages =
not a fan, i think the original version is much easier to read
File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/transactions/

Line 28:     public static final String KEY_MARKER_CALLBACK = "MARKER_CALLBACK";
Should we remove the redundant modifiers (public static final) given that this is a new interface?

Line 36:     public void before(ByteBuffer buffer);
remove public

Line 43:     public void after(long lsn);
remove public
File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/transactions/

Line 258:         switch (logType) {
> MAJOR SonarQube violation:
Can we restore the default case?  Even if a no-op, this is good documentation that other case
should do nothing.  If we are handling all cases, we should add a default case that throws
File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/utils/

Line 92:                 file.getParentFile().mkdirs();
Isn't the issue still present, (reported in patch 7)
File hyracks-fullstack/hyracks/hyracks-dataflow-common/src/main/java/org/apache/hyracks/dataflow/common/io/

Line 119:             int messageSize = message.getBuffer().limit() - message.getBuffer().position();
Can we add a local final variable for message.getBuffer()?  Seems nice since we call it 3
times (from 4 places).
File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/

Line 476:             return (metadataPageNum * bufferCache.getPageSize()) + LIFOMetaDataFrame.LSN_OFFSET;
> CRITICAL SonarQube violation:
+1- I would cast bufferCache.getPageSize() to long

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I3b9aa8de758b7d26ca34868b16e5ce693e0c0243
Gerrit-PatchSet: 10
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <>
Gerrit-Reviewer: Jenkins <>
Gerrit-Reviewer: Michael Blow <>
Gerrit-Reviewer: Till Westmann <>
Gerrit-Reviewer: abdullah alamoudi <>
Gerrit-HasComments: Yes

View raw message