Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/16204 )
Change subject: IMPALA-8125: Add query option to limit number of hdfs writer instances ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/16204/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16204/3//COMMIT_MSG@24 PS3, Line 24: - Adding planner tests to cover all cases where this enforcement kicks nit: 'Added' http://gerrit.cloudera.org:8080/#/c/16204/3/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java File fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java: http://gerrit.cloudera.org:8080/#/c/16204/3/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java@214 PS3, Line 214: List<ScanNode> hdfsScanORUnionNodes = Lists.newArrayList(); It's not completely clear why this checks for Union. Scan makes sense since it is a leaf fragment. But Union could be considered an 'internal or root fragment' no ? Also, it would be good to abstract this into a separate function .. something like isInternalFragment() that could be part of the PlanFragment class. http://gerrit.cloudera.org:8080/#/c/16204/3/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java@241 PS3, Line 241: // MAX_HDFS_WRITER query option. nit: max_fs_writers http://gerrit.cloudera.org:8080/#/c/16204/3/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java@277 PS3, Line 277: // legacy behavior(when not using MAX_HDFS_WRITER query option). nit: max_fs_writers -- To view, visit http://gerrit.cloudera.org:8080/16204 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17c8e61b9a32d908eec82c83618ff9caa41078a5 Gerrit-Change-Number: 16204 Gerrit-PatchSet: 3 Gerrit-Owner: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Fri, 24 Jul 2020 06:56:18 +0000 Gerrit-HasComments: Yes
