>From Murtadha Hubail <[email protected]>: Attention is currently required from: Ian Maxon, Ritik Raj.
Murtadha Hubail has posted comments on this change by Ritik Raj. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959?usp=email ) Change subject: [ASTERIXDB-3702][RT][STO] LSM Sampling ...................................................................... Patch Set 19: (21 comments) File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/e8f23563_f757ce63?usp=email : PS19, Line 5239: warn info? File asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/statement/AnalyzeStatement.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/393f4ab5_5ba6bb16?usp=email : PS19, Line 185: fullScanNode.getType() != ATypeTag.BOOLEAN) Will we fail here if it's "true"? because the earlier error message during validation says string is fine. File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entitytupletranslators/IndexTupleTranslator.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/ed5570b6_99e7f074?usp=email : PS19, Line 103: SampleIsFullScan Sleep on this as we can't change it later https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/b9e5f09e_3c656427?usp=email : PS19, Line 521: isFullScan This should default to true File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/profiling/OperatorStats.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/c40fc1da_3c1d5435?usp=email : PS19, Line 275: : public ICounter getCloudReadPageCounter() { : return cloudReadPageCounter; : } : : public ICounter getColdReadCounter() { : return coldReadCounter; : } : Remove if not needed File hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/api/IBTreeInteriorFrame.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/d594fda3_d97763db?usp=email : PS19, Line 34: : public int getRightLeafOffset(); : : public ITreeIndexTupleReference getTupleRef(); Check if you need to add those or reuse existing APIs File hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTree.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/98389717_7512610f?usp=email : PS19, Line 987: IComponentSampler Doesn't look like BTree should be aware of components File hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTreeCursorInitialState.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/5023020d_41b1e692?usp=email : PS19, Line 62: public void setRootPageId(int rootPageId) { : this.rootPageId = rootPageId; : } I don't like this setting. See if you can reuse the existing APIs File hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/DiskBTree.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/25141fa6_827b49ec?usp=email : PS19, Line 147: getRandomLeafPage This is not used! File hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/DiskBTreeSampleCursor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/e97617c1_b3f77ea9?usp=email : PS19, Line 90: rootPageId this can be local File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ILSMIndexBatchPointCursor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/665709b5_dd7ef2bc?usp=email : PS19, Line 29: // Reuses same sample cursor for the predicates Use javadocs https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/c485a56c_d9bd0f6d?usp=email : PS19, Line 32: doHasNextWithPredicate do? File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree-column/src/main/java/org/apache/hyracks/storage/am/lsm/btree/column/dataflow/LSMColumnBTreeLocalResource.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/a1856c7f_fdd4c2b4?usp=email : PS19, Line 96: LSMBTree lsmBTree = (LSMBTree) ? File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree-column/src/main/java/org/apache/hyracks/storage/am/lsm/btree/column/impls/btree/ColumnBtreeSampleCursor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/ce64315a_1d9adc43?usp=email : PS19, Line 296: refillLeafDrawBatch Maybe can refactor? https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/cb960741_6763d01c?usp=email : PS19, Line 364: long nanos = traceTimingEnabled ? System.nanoTime() : 0L; Do we still need to keep this time tracing? Same for the others File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/LSMBTreeLocalResource.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/a0d0a1a9_0bfbf17d?usp=email : PS19, Line 178: getOrDefaultInt not used File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMIndexSampleCursor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/7277c395_d2dc01ec?usp=email : PS19, Line 170: computeDiskComponentSampleProportionality This method is took look. See if you can refactor it to logical units https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/13065f01_c22ef77e?usp=email : PS19, Line 256: warn don't warn on this File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/DiskComponentMetadata.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/cdecf0c7_f08bdb93?usp=email : PS19, Line 32: THETA_INSERT_DELETE_SKETCH_KEY Sleep on those File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/theta/ThetaSampler.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/ace89af6_c862e0ae?usp=email : PS19, Line 190: serialize Just moving the method logic here File hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-invertedindex-test/src/test/java/org/apache/hyracks/storage/am/lsm/invertedindex/util/LSMInvertedIndexTestUtils.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959/comment/c2321614_22806237?usp=email : PS19, Line 262: org.apache.hyracks.storage.common. Chat with your Claude -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20959?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: If5c5b7eac5199b85fc40b722c87ae0bbb73eecbd Gerrit-Change-Number: 20959 Gerrit-PatchSet: 19 Gerrit-Owner: Ritik Raj <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-CC: Ian Maxon <[email protected]> Gerrit-CC: Murtadha Hubail <[email protected]> Gerrit-Attention: Ian Maxon <[email protected]> Gerrit-Attention: Ritik Raj <[email protected]> Gerrit-Comment-Date: Fri, 10 Jul 2026 13:37:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No
