asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yingyi Bu (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: ASTERIXDB-1566, ASTERIXDB-1733: Hash Group By and Hash Join ...
Date Sat, 31 Dec 2016 00:32:44 GMT
Yingyi Bu 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)

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.


Line 66:     <value>256KB</value>
keep the number unchanged.


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.


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 298:     public static int calculateGroupByTableCardinality(int memoryBudgetInBytes,
int numberOfGroupByColumns,
public -> private


Line 313:         long possibleNumberOfHashEntries = (long) 2 << numberOfBits;
(long) 2 << numberOfBits  

->

2L << numberOfBits


Line 317:         long groupByTableByteSize = SerializableHashTable.getExpectedTableSizeInByte((int)
groupByTableSize, frameSize);
data loss?


https://asterix-gerrit.ics.uci.edu/#/c/1056/54/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/structures/SerializableHashTable.java
File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/structures/SerializableHashTable.java:

Line 44:     private static final Logger LOGGER = Logger.getLogger(SerializableHashTable.class.getName());
> MAJOR SonarQube violation:
address this.


https://asterix-gerrit.ics.uci.edu/#/c/1056/54/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/structures/SimpleSerializableHashTable.java
File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/structures/SimpleSerializableHashTable.java:

Line 444:     public static int getExpectedTableSizeInFrame(int tableSize, int frameSize)
{
this should be a long?


Line 447:                 .ceil(((double) getNumberOfEntryInSlot() * 2 * getUnitSize() * tableSize)
/ (double) frameSize));
it seems that it's not necessary to go through the double detour.  Using long/integers and
ignoring things <1 would be fine?


Line 490:             bytes[offset++] = (byte) (value);
> MAJOR SonarQube violation:
address this.


-- 
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 <wangsaeu@yahoo.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Jianfeng Jia <jianfeng.jia@gmail.com>
Gerrit-Reviewer: Michael Blow <mblow@apache.org>
Gerrit-Reviewer: Steven Jacobs <sjaco002@ucr.edu>
Gerrit-Reviewer: Taewoo Kim <wangsaeu@yahoo.com>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: Wenhai Li <lwhaymail@yahoo.com>
Gerrit-Reviewer: Yingyi Bu <buyingyi@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message