Taewoo Kim has posted comments on this change. Change subject: ASTERIXDB-1566, ASTERIXDB-1733: Hash Group By and Hash Join conform to the memory budget ......................................................................
Patch Set 54: (10 comments) Thanks Yingyi. https://asterix-gerrit.ics.uci.edu/#/c/1056/54/asterixdb/asterix-app/src/main/resources/asterix-build-configuration.xml File asterixdb/asterix-app/src/main/resources/asterix-build-configuration.xml: Line 62: <value>256KB</value> > keep the number unchanged. Done Line 66: <value>256KB</value> > keep the number unchanged. Based on our conversation, we keep this in order to deal with the big object cases. https://asterix-gerrit.ics.uci.edu/#/c/1056/54/asterixdb/asterix-app/src/test/resources/runtimets/results/api/cluster_state_1/cluster_state_1.1.adm File asterixdb/asterix-app/src/test/resources/runtimets/results/api/cluster_state_1/cluster_state_1.1.adm: Line 16: "compiler.groupmemory": 262144, > Ideally, those two numbers should be unchanged. For groupmemory, yes. For join memory, we will keep this number based on our conversation. https://asterix-gerrit.ics.uci.edu/#/c/1056/54/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/ExternalGroupByPOperator.java File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/ExternalGroupByPOperator.java: Line 297: */ > Add a unit test for this method? (It feels that this is a place where thin Done Line 298: public static int calculateGroupByTableCardinality(int memoryBudgetInBytes, int numberOfGroupByColumns, > public -> private Done Line 313: long possibleNumberOfHashEntries = (long) 2 << numberOfBits; > (long) 2 << numberOfBits Done Line 317: long groupByTableByteSize = SerializableHashTable.getExpectedTableSizeInByte((int) groupByTableSize, frameSize); > data loss? Changed the logic. https://asterix-gerrit.ics.uci.edu/#/c/1056/54/hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/unit/AbstractExternalGroupbyTest.java File hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/unit/AbstractExternalGroupbyTest.java: Line 179: int numFrames = 50; > keep 3 as unchanged? This test can't run well when you count the hash table size for 1001 elements. The number of header frames (which will not be shrink at all under any circumstance) is already 8008 bytes / 256 bytes = 32 frames. Even for this 101 entry, 101 * 4 * 2 = 808 / 256 bytes = 4 frames. I think it's reasonable to keep this number. We can test all cases. Line 190: int numFrames = 50; > keep 3 unchanged? the same as the above. Line 201: int numFrames = 50; > keep 3 unchanged? the same as the above. -- To view, visit https://asterix-gerrit.ics.uci.edu/1056 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2b323e9a2141b4c1dd1652a360d2d9354d3bc3f5 Gerrit-PatchSet: 54 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Taewoo Kim <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Jianfeng Jia <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Steven Jacobs <[email protected]> Gerrit-Reviewer: Taewoo Kim <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Wenhai Li <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-HasComments: Yes
