[GitHub] drill issue #647: DRILL-4935 Allow Drill to advertise a specific hostname to...
Github user harrisonmebane commented on the issue: https://github.com/apache/drill/pull/647 Do I need to do anything more to push this forward? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #647: DRILL-4935 Allow Drill to advertise a specific hostname to...
Github user harrisonmebane commented on the issue: https://github.com/apache/drill/pull/647 I have tested this fix manually, in the following way: * Deploy Drill in Docker containers on an existing cluster on AWS, with the line ```export DRILL_HOST_NAME=`curl http://169.254.169.254/latest/meta-data/local-ipv4` ``` in `drill-env.sh`, to ensure that the variable is populated with the host machine's IP. * Start a Drill session in the docker container and run `select * from sys.drillbits;`. The result was: ``` +++---++--+ |hostname| user_port | control_port | data_port | current | +++---++--+ | 172.31.21.207 | 31010 | 31011 | 31012 | false| | 172.31.29.130 | 31010 | 31011 | 31012 | true | | 172.31.22.200 | 31010 | 31011 | 31012 | false| +++---++--+ ``` * On one of the drillbits, comment out the `DRILL_HOST_NAME` line in `drill-env.sh`, unset the variable, and restart the drillbit. * Log into Drill shell again and run `select * from sys.drillbits;` Result is now: ``` +++---++--+ |hostname| user_port | control_port | data_port | current | +++---++--+ | 172.31.22.200 | 31010 | 31011 | 31012 | false| | a53b37888f62 | 31010 | 31011 | 31012 | true | | 172.31.29.130 | 31010 | 31011 | 31012 | false| +++---++--+ ``` * Running queries that require the other nodes results in a `Error: SYSTEM ERROR: UnresolvedAddressException`, indicating that the new address registered on Zookeeper no longer works --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #647: DRILL-4935 Allow Drill to advertise a specific hostname to...
Github user harrisonmebane commented on the issue: https://github.com/apache/drill/pull/647 I have made the changes suggested by @paul-rogers . Are there any unit tests we should add? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #647: DRILL-4935 Allow Drill to advertise a specific host...
Github user harrisonmebane commented on a diff in the pull request: https://github.com/apache/drill/pull/647#discussion_r86922289 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java --- @@ -49,6 +49,7 @@ String USER_RPC_TIMEOUT = "drill.exec.rpc.user.timeout"; String METRICS_CONTEXT_NAME = "drill.exec.metrics.context"; String USE_IP_ADDRESS = "drill.exec.rpc.use.ip"; + String BIT_ADVERTISED_HOST = "drill.exec.rpc.bit.advertised.host"; --- End diff -- That all seems reasonable to me. I can't find any examples of environment variables being accessed from in the Java code. I assume we don't want the name of the environment variable hard-coded in the hostname resolution code, but I'm not sure of the best place to define it, i.e. ```String DRILL_HOST_NAME = "DRILL_HOST_NAME"``` Thoughts? I could put it in `ExecConstants` but it would be the only env variable in there. I could also just make it a static variable in the `ServiceEngine` class. Do we still want to provide the option to override through a system property, or just rely on the environment variable? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #647: DRILL-4935 Allow Drill to advertise a specific host...
Github user harrisonmebane commented on a diff in the pull request: https://github.com/apache/drill/pull/647#discussion_r86883806 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java --- @@ -49,6 +49,7 @@ String USER_RPC_TIMEOUT = "drill.exec.rpc.user.timeout"; String METRICS_CONTEXT_NAME = "drill.exec.metrics.context"; String USE_IP_ADDRESS = "drill.exec.rpc.use.ip"; + String BIT_ADVERTISED_HOST = "drill.exec.rpc.bit.advertised.host"; --- End diff -- @paul-rogers Thank you for your helpful suggestions. I understand what you are saying, and indeed, I was using exactly the flow you outlined above for my own use case. Let me make sure I understand what you are proposing. We can either: 1. Access the host name as an environment variable, in which case we could access it via `System.getEnv(...)` 2. Pass the host name in as a system variable (``-Ddrill.exec.rpc.bit.advertised-host=myhost`), which could just override the parameter I've already specified in `ExecConstants.java` In either case, I guess I don't understand why I'd need to add anything to `drill-env.sh`, unless you mean just to add a commented out line for documentation purposes. In the second case, is there any change I would need to make at all? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #647: DRILL-4935 Allow Drill to advertise a specific host...
Github user harrisonmebane commented on a diff in the pull request:
https://github.com/apache/drill/pull/647#discussion_r86856157
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/service/ServiceEngine.java
---
@@ -142,7 +142,13 @@ private static BufferAllocator newAllocator(
public DrillbitEndpoint start() throws DrillbitStartupException,
UnknownHostException{
int userPort =
userServer.bind(config.getInt(ExecConstants.INITIAL_USER_PORT),
allowPortHunting);
-String address = useIP ? InetAddress.getLocalHost().getHostAddress()
: InetAddress.getLocalHost().getCanonicalHostName();
+String configIP = config.getString(ExecConstants.BIT_ADVERTISED_HOST);
+String address = null;
+if (configIP == "") {
--- End diff --
Good suggestion! Much cleaner.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill issue #647: DRILL-4935 Allow Drill to advertise a specific hostname to...
Github user harrisonmebane commented on the issue: https://github.com/apache/drill/pull/647 I have implemented @xhochy 's suggestion. I will need some guidance on the best way to unit test this new configuration. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #647: DRILL-4935 Allow Drill to advertise a specific host...
GitHub user harrisonmebane opened a pull request: https://github.com/apache/drill/pull/647 DRILL-4935 Allow Drill to advertise a specific hostname to Zookeeper You can merge this pull request into a Git repository by running: $ git pull https://github.com/harrisonmebane/drill DRILL-4935 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/647.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 #647 commit 01dd583ff0f7dc0092371a85823b4737ca26b02d Author: Harrison Mebane <harri...@svds.com> Date: 2016-10-07T20:10:36Z allow configuration of advertised drillbit IP address --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
