>From Ian Maxon <[email protected]>: Attention is currently required from: Hongyu Shi.
Ian Maxon has posted comments on this change by Hongyu Shi. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21100?usp=email ) Change subject: [ASTERIXDB-3754] Storage layer of VTree index - patch 2 ...................................................................... Patch Set 5: (6 comments) File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-vtree/src/main/java/org/apache/hyracks/storage/am/lsm/vector/dataflow/LSMVTreeLocalResource.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21100/comment/aea22dfe_7151be76?usp=email : PS5, Line 67: ".quantization_ maybe this should be in AbstractLSMIndexFileManager https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21100/comment/a7dd6091_9bc0b088?usp=email : PS5, Line 226: tryReadQuantizationSidecarFile( it seems fine but wouldn't it be easier to put this in the JSON? that way if you have to change or extend this at some point, it will be simpler to maintain backwards compatibility. unless there's some perf reason https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21100/comment/ca3f6b02_b24ba45d?usp=email : PS5, Line 383: // ==================== Public Getter Methods for Quantization Parameters ==================== kinda excessive comment maybe File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-vtree/src/main/java/org/apache/hyracks/storage/am/lsm/vector/dataflow/QuantizedIndexCreateOperatorDescriptor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21100/comment/5856f8c7_d9194119?usp=email : PS5, Line 91: System.err.println("[QuantizedIndexCreateOp] nextFrame() - received frame with " + tupleCount + " tuples"); if these statements using System.err need to be logged they need to be through LOGGER.trace or something File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-vtree/src/main/java/org/apache/hyracks/storage/am/lsm/vector/dataflow/VectorSearchOperatorNodePushable.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21100/comment/794aebe1_c259b662?usp=email : PS5, Line 107: // Note: this comment block is kind of confusing from the Note: prefix on each line File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-vtree/src/main/java/org/apache/hyracks/storage/am/lsm/vector/impls/LSMVTreeDiskComponent.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21100/comment/2e3766d7_eeb41c98?usp=email : PS5, Line 156: numLevels = (Integer) parameters.getOrDefault("numLevels", 2); : @SuppressWarnings("unchecked") : List<Integer> clustersPerLevel = : (List<Integer>) parameters.getOrDefault("clustersPerLevel", Arrays.asList(5, 10)); would it be expected to somehow get a null parameter here normally? i worry about what might happen if it does somehow become null, and silently takes these params. would it cause issues? -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21100?usp=email To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: I87c468c14cbeb3e698742429ee62df02c8e83efe Gerrit-Change-Number: 21100 Gerrit-PatchSet: 5 Gerrit-Owner: Hongyu Shi <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-CC: Ian Maxon <[email protected]> Gerrit-Attention: Hongyu Shi <[email protected]> Gerrit-Comment-Date: Mon, 18 May 2026 18:02:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No
