Bikramjeet Vig 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 4: (5 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: - Added planner tests to cover all cases where this enforcement kicks > nit: 'Added' Done 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: // We also consider fragments containing union nodes along with scan fragments > It's not completely clear why this checks for Union. Scan makes sense since Added a comment to explain why we are considering union nodes. I skipped adding a separate function since scans are leaf fragments and unions may or may not be so it would get confusing if i integrated that logic separately in the PlanFragment class. Moreover the comment at L230 adds more context to this which would also justify keeping it here. Open to suggestions so let me know if you feel strongly about this. http://gerrit.cloudera.org:8080/#/c/16204/3/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java@236 PS3, Line 236: of > nit: "to" seems misplaced. Done http://gerrit.cloudera.org:8080/#/c/16204/3/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java@241 PS3, Line 241: int input_instances = inputFragment.getNumInstances(); > nit: max_fs_writers Done http://gerrit.cloudera.org:8080/#/c/16204/3/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java@277 PS3, Line 277: if (enforce_hdfs_writer_limit > nit: max_fs_writers Done -- 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: 4 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: Mon, 27 Jul 2020 20:07:28 +0000 Gerrit-HasComments: Yes
