You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by ilooner <gi...@git.apache.org> on 2018/02/09 05:14:19 UTC
[GitHub] drill pull request #1119: DRILL-6143: Made FragmentsRunner's rpc timeout lar...
GitHub user ilooner opened a pull request:
https://github.com/apache/drill/pull/1119
DRILL-6143: Made FragmentsRunner's rpc timeout larger to reduce rando…
…m failures and made it configurable as a SystemOption.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ilooner/drill DRILL-6143
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/1119.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1119
----
commit d918265c4d5caee11c0b707ff49a49c547c8dc8a
Author: Timothy Farkas <ti...@...>
Date: 2018-02-08T23:25:59Z
DRILL-6143: Made FragmentsRunner's rpc timeout larger to reduce random failures and made it configurable as a SystemOption.
----
---
[GitHub] drill issue #1119: DRILL-6143: Made FragmentsRunner's rpc timeout a SystemOp...
Posted by priteshm <gi...@git.apache.org>.
Github user priteshm commented on the issue:
https://github.com/apache/drill/pull/1119
@arina-ielchiieva is on vacation. @vrozov, @Ben-Zvi can you take a look?
---
[GitHub] drill issue #1119: DRILL-6143: Made FragmentsRunner's rpc timeout a SystemOp...
Posted by ilooner <gi...@git.apache.org>.
Github user ilooner commented on the issue:
https://github.com/apache/drill/pull/1119
@arina-ielchiieva
---
[GitHub] drill pull request #1119: DRILL-6143: Made FragmentsRunner's rpc timeout a S...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on a diff in the pull request:
https://github.com/apache/drill/pull/1119#discussion_r167312527
--- Diff: exec/java-exec/src/main/resources/drill-module.conf ---
@@ -413,6 +413,7 @@ drill.exec.options: {
# to start at least 2 partitions then HashAgg fallbacks to this case. It can be
# enabled by setting this flag to true. By default it's set to false such that
# query will fail if there is not enough memory
+ drill.exec.rpc.fragrunner.timeout: 30000,
--- End diff --
The value looks a little bit high and it needs to be moved either below `drill.exec.hashagg.fallback.enabled` or above the preceding comment, otherwise LGTM.
---
[GitHub] drill pull request #1119: DRILL-6143: Made FragmentsRunner's rpc timeout a S...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/drill/pull/1119
---
[GitHub] drill pull request #1119: DRILL-6143: Made FragmentsRunner's rpc timeout a S...
Posted by Ben-Zvi <gi...@git.apache.org>.
Github user Ben-Zvi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1119#discussion_r167343642
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java ---
@@ -212,7 +212,8 @@
new OptionDefinition(ExecConstants.CPU_LOAD_AVERAGE),
new OptionDefinition(ExecConstants.ENABLE_VECTOR_VALIDATOR),
new OptionDefinition(ExecConstants.ENABLE_ITERATOR_VALIDATOR),
- new OptionDefinition(ExecConstants.OUTPUT_BATCH_SIZE_VALIDATOR, new OptionMetaData(OptionValue.AccessibleScopes.SYSTEM, true, false))
+ new OptionDefinition(ExecConstants.OUTPUT_BATCH_SIZE_VALIDATOR, new OptionMetaData(OptionValue.AccessibleScopes.SYSTEM, true, false)),
+ new OptionDefinition(ExecConstants.FRAG_RUNNER_RPC_TIMEOUT_VALIDATOR, new OptionMetaData(OptionValue.AccessibleScopes.SYSTEM, false, true)),
--- End diff --
Question: Should the last two parameters be instead:
adminOnly = true
internal = false
---
[GitHub] drill issue #1119: DRILL-6143: Made FragmentsRunner's rpc timeout a SystemOp...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the issue:
https://github.com/apache/drill/pull/1119
LGTM
---
[GitHub] drill pull request #1119: DRILL-6143: Made FragmentsRunner's rpc timeout a S...
Posted by ilooner <gi...@git.apache.org>.
Github user ilooner commented on a diff in the pull request:
https://github.com/apache/drill/pull/1119#discussion_r167338469
--- Diff: exec/java-exec/src/main/resources/drill-module.conf ---
@@ -413,6 +413,7 @@ drill.exec.options: {
# to start at least 2 partitions then HashAgg fallbacks to this case. It can be
# enabled by setting this flag to true. By default it's set to false such that
# query will fail if there is not enough memory
+ drill.exec.rpc.fragrunner.timeout: 30000,
--- End diff --
Thanks for catching the ordering. I reduced the default to 10000.
---
[GitHub] drill pull request #1119: DRILL-6143: Made FragmentsRunner's rpc timeout a S...
Posted by ilooner <gi...@git.apache.org>.
Github user ilooner commented on a diff in the pull request:
https://github.com/apache/drill/pull/1119#discussion_r167346417
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java ---
@@ -212,7 +212,8 @@
new OptionDefinition(ExecConstants.CPU_LOAD_AVERAGE),
new OptionDefinition(ExecConstants.ENABLE_VECTOR_VALIDATOR),
new OptionDefinition(ExecConstants.ENABLE_ITERATOR_VALIDATOR),
- new OptionDefinition(ExecConstants.OUTPUT_BATCH_SIZE_VALIDATOR, new OptionMetaData(OptionValue.AccessibleScopes.SYSTEM, true, false))
+ new OptionDefinition(ExecConstants.OUTPUT_BATCH_SIZE_VALIDATOR, new OptionMetaData(OptionValue.AccessibleScopes.SYSTEM, true, false)),
+ new OptionDefinition(ExecConstants.FRAG_RUNNER_RPC_TIMEOUT_VALIDATOR, new OptionMetaData(OptionValue.AccessibleScopes.SYSTEM, false, true)),
--- End diff --
internal should be true since we want this to show up in the internal options table and not the standard system options table. Changing to adminOnly = true seems reasonable.
---