abdullah alamoudi 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); : } > Why's this different for RTree/BTree/Inverted though? The part I don't get Oh, because we never set it for Inverted Index, or for RTree. And this is different from the LSN the LSN points to the LSN of the first log of the transaction log for the component. While this one points to the LSN of a marker log that allows the system to move through the transaction log backward in order to do rollbacks. 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. Done 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 mes Hmmm, it is basically trying to open One metadata disk component and look for this key value pair in its metadata page. So, it is either an IO error or a bug!! This call however is on the log flusher path and it doesn't expect any errors. hence, the method signature doesn't have checked exceptions. An exception simply means that the system is in an inconsistent state since the log flusher will stop. 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]; : } > Sure, but is that really the intended behavior, that you should call setLon usually what we do is that we create the byte array and set it from outside then call setLong(bytes, start, value). I've seen this enough times I thought, I would make this more convenient 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()) because what we need is range equality and not the whole array. 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?.. This fixes a different bug in our web interface. psst don't tell anyone.!! Next time I will create a different change but now, this is convenient :D -- 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
