Ian Maxon has posted comments on this change. Change subject: Improve reading from and writing to Metadata pages ......................................................................
Patch Set 12: (6 comments) https://asterix-gerrit.ics.uci.edu/#/c/1476/2/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/ioopcallbacks/LSMBTreeIOOperationCallback.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/ioopcallbacks/LSMBTreeIOOperationCallback.java: PS2, Line 48: if (opType == LSMOperationType.MERGE) { : LongPointable markerLsn = LongPointable.FACTORY.createPointable(ComponentMetadataUtil : .getLong(oldComponents.get(0).getMetadata(), ComponentMetadataUtil.MARKER_LSN_KEY, -1L)); : btreeComponent.getMetadata().put(ComponentMetadataUtil.MARKER_LSN_KEY, markerLsn); : } > this used to be inside the merge method call. Basically, it takes the lsn o Why's this different for RTree/BTree/Inverted though? The part I don't get is why it isn't in putLSNIntoMetadata https://asterix-gerrit.ics.uci.edu/#/c/1476/12/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/ioopcallbacks/LSMBTreeIOOperationCallback.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/ioopcallbacks/LSMBTreeIOOperationCallback.java: PS12, Line 50: -1 Using NOT_FOUND here would be good as well for clarity. https://asterix-gerrit.ics.uci.edu/#/c/1476/12/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/transactions/PrimaryIndexLogMarkerCallback.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/transactions/PrimaryIndexLogMarkerCallback.java: PS12, Line 85: Unable to read metadata page. Disk Error? This could be migrated to the error codes couldn't it? I also think the message isn't so great. It's more like, something went wrong when trying to scan disk components for LSNs. It doesn't necessarily indicate anything else. https://asterix-gerrit.ics.uci.edu/#/c/1476/2/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/LongPointable.java File hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/LongPointable.java: PS2, Line 91: if (bytes == null) { : start = 0; : length = TYPE_TRAITS.getFixedLength(); : bytes = new byte[length]; : } > otherwise, it will throw a nullpointer exception. Sure, but is that really the intended behavior, that you should call setLong before anything else that would otherwise set bytes to be non-null? https://asterix-gerrit.ics.uci.edu/#/c/1476/12/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/GrowableArray.java File hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/GrowableArray.java: PS12, Line 89: int length = baaos.getLength(); : if (other.baaos.getLength() != length) { : return false; : } : byte[] array1 = baaos.getByteArray(); : byte[] array2 = other.baaos.getByteArray(); : for (int i = 0; i < length; i++) { : if (array1[i] != array2[i]) { : return false; : } : } : return true; Why not just Arrays.equals(baaos.getByteArray(),other.baaos.getByteArray()); ? https://asterix-gerrit.ics.uci.edu/#/c/1476/12/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/ChunkedNettyOutputStream.java File hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/ChunkedNettyOutputStream.java: PS12, Line 76: lic void close() throws IOException { : if (!closed) { : if (response.isHeaderSent() || response.status() != HttpResponseStatus.OK) { : flush(); : buffer.release(); : } else { : response.fullReponse(buffer); : } : super.close(); : } : closed = true; This is unrelated?.. -- To view, visit https://asterix-gerrit.ics.uci.edu/1476 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id95ef33c0a0bc1abb3fc3ecdea5611ee4acd6dfa Gerrit-PatchSet: 12 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
