You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hawq.apache.org by jiny2 <gi...@git.apache.org> on 2017/01/10 02:35:27 UTC
[GitHub] incubator-hawq pull request #1078: HAWQ-1258. segment resource manager does ...
GitHub user jiny2 opened a pull request:
https://github.com/apache/incubator-hawq/pull/1078
HAWQ-1258. segment resource manager does not switch back when it cannot resolve standby host name
If use does not configure standby host, segment will not switch to standby server; If segment cannot resolve target server no matter the master server or the standby server, the target server for sending IMAlive heart-beat is switched as well.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jiny2/incubator-hawq HAWQ-1258
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hawq/pull/1078.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 #1078
----
commit 67ebb204e65a2b65b48acc75db0a41332858d688
Author: Yi <yj...@pivotal.io>
Date: 2017-01-10T02:33:16Z
HAWQ-1258. segment resource manager does not switch back when it cannot resolve standby host name
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #1078: HAWQ-1258. segment resource manager does ...
Posted by jiny2 <gi...@git.apache.org>.
Github user jiny2 commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1078#discussion_r95294321
--- Diff: src/backend/utils/misc/guc.c ---
@@ -8200,7 +8200,7 @@ static struct config_string ConfigureNamesString[] =
NULL
},
&standby_addr_host,
- "localhost", NULL, NULL
+ "none", NULL, NULL
--- End diff --
That needs @radarwave 's comment, I have similar question, but currently from script implementation, "none" means no standby.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #1078: HAWQ-1258. segment resource manager does ...
Posted by jiny2 <gi...@git.apache.org>.
Github user jiny2 commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1078#discussion_r95706815
--- Diff: src/backend/utils/misc/guc.c ---
@@ -8200,7 +8200,7 @@ static struct config_string ConfigureNamesString[] =
NULL
},
&standby_addr_host,
- "localhost", NULL, NULL
+ "none", NULL, NULL
--- End diff --
https://issues.apache.org/jira/browse/HAWQ-1266 this jira is created for discussion. Thanks @paul-guo- 's thorough think.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #1078: HAWQ-1258. segment resource manager does ...
Posted by paul-guo- <gi...@git.apache.org>.
Github user paul-guo- commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1078#discussion_r95294635
--- Diff: src/backend/utils/misc/guc.c ---
@@ -8200,7 +8200,7 @@ static struct config_string ConfigureNamesString[] =
NULL
},
&standby_addr_host,
- "localhost", NULL, NULL
+ "none", NULL, NULL
--- End diff --
I do not think the code should be bundled with the python utility code. That will possibly introduce some potential bugs in the future.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #1078: HAWQ-1258. segment resource manager does ...
Posted by paul-guo- <gi...@git.apache.org>.
Github user paul-guo- commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1078#discussion_r95314993
--- Diff: src/backend/utils/misc/guc.c ---
@@ -8200,7 +8200,7 @@ static struct config_string ConfigureNamesString[] =
NULL
},
&standby_addr_host,
- "localhost", NULL, NULL
+ "none", NULL, NULL
--- End diff --
Not sure whether I understand correctly, but could not we modify the code like this?
"", NULL, NULL
if (standby_addr_host[0] == '\0')
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #1078: HAWQ-1258. segment resource manager does ...
Posted by jiny2 <gi...@git.apache.org>.
Github user jiny2 closed the pull request at:
https://github.com/apache/incubator-hawq/pull/1078
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq issue #1078: HAWQ-1258. segment resource manager does not swi...
Posted by linwen <gi...@git.apache.org>.
Github user linwen commented on the issue:
https://github.com/apache/incubator-hawq/pull/1078
LGTM
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq issue #1078: HAWQ-1258. segment resource manager does not swi...
Posted by jiny2 <gi...@git.apache.org>.
Github user jiny2 commented on the issue:
https://github.com/apache/incubator-hawq/pull/1078
@paul-guo- @linwen Please review, thanks!
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #1078: HAWQ-1258. segment resource manager does ...
Posted by jiny2 <gi...@git.apache.org>.
Github user jiny2 commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1078#discussion_r95513882
--- Diff: src/backend/utils/misc/guc.c ---
@@ -8200,7 +8200,7 @@ static struct config_string ConfigureNamesString[] =
NULL
},
&standby_addr_host,
- "localhost", NULL, NULL
+ "none", NULL, NULL
--- End diff --
This will cause behavior inconsistency, when "none" is set script thinks there is no standby, but rm segment will try to resolve "none", which is not necessary.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #1078: HAWQ-1258. segment resource manager does ...
Posted by paul-guo- <gi...@git.apache.org>.
Github user paul-guo- commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1078#discussion_r95293627
--- Diff: src/backend/resourcemanager/communication/rmcomm_RMSEG2RM.c ---
@@ -107,8 +107,10 @@ int sendIMAlive(int *errorcode,
if ( res != FUNC_RETURN_OK )
{
rm_pfree(AsyncCommContext, context);
- elog(WARNING, "Fail to register asynchronous connection for sending "
- "IMAlive message. %d", res);
+ elog(LOG, "failed to register asynchronous connection for sending "
+ "IMAlive message. %d", res);
+ /* Always switch if fail to register connection here. */
+ switchIMAliveSendingTarget();
--- End diff --
How often is switchIMAliveSendingTarget()? I mean if it is called often, we should have a parameter or bit mask to save the info that standby exists or not to avoid frequent string comparison which is obviously heavier than simple logical operations.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #1078: HAWQ-1258. segment resource manager does ...
Posted by radarwave <gi...@git.apache.org>.
Github user radarwave commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1078#discussion_r95525426
--- Diff: src/backend/utils/misc/guc.c ---
@@ -8200,7 +8200,7 @@ static struct config_string ConfigureNamesString[] =
NULL
},
&standby_addr_host,
- "localhost", NULL, NULL
+ "none", NULL, NULL
--- End diff --
Yeah, it's not so good to use 'none' to indicate there is no standby. But I don't think use '' or ' ' or ' ' are good too. Still let user feel confusion. Maybe we can add a new guc to indicate if we have a standby master defined, the value can be yes/no.
Better to start a new Jira to talk about this.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq issue #1078: HAWQ-1258. segment resource manager does not swi...
Posted by stanlyxiang <gi...@git.apache.org>.
Github user stanlyxiang commented on the issue:
https://github.com/apache/incubator-hawq/pull/1078
+1
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #1078: HAWQ-1258. segment resource manager does ...
Posted by jiny2 <gi...@git.apache.org>.
Github user jiny2 commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1078#discussion_r95522787
--- Diff: src/backend/utils/misc/guc.c ---
@@ -8200,7 +8200,7 @@ static struct config_string ConfigureNamesString[] =
NULL
},
&standby_addr_host,
- "localhost", NULL, NULL
+ "none", NULL, NULL
--- End diff --
No, segment rm process does not care what are saved in python code. So, it only knows there is a standby host named "none". So deleting from hawq_dict does not help.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #1078: HAWQ-1258. segment resource manager does ...
Posted by jiny2 <gi...@git.apache.org>.
Github user jiny2 commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1078#discussion_r95310043
--- Diff: src/backend/utils/misc/guc.c ---
@@ -8200,7 +8200,7 @@ static struct config_string ConfigureNamesString[] =
NULL
},
&standby_addr_host,
- "localhost", NULL, NULL
+ "none", NULL, NULL
--- End diff --
If we support "" instead of "none", we have segment keep connecting to "none" as a server. I think if necessary, we need another fix for this.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #1078: HAWQ-1258. segment resource manager does ...
Posted by paul-guo- <gi...@git.apache.org>.
Github user paul-guo- commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1078#discussion_r95294534
--- Diff: src/backend/resourcemanager/communication/rmcomm_RMSEG2RM.c ---
@@ -107,8 +107,10 @@ int sendIMAlive(int *errorcode,
if ( res != FUNC_RETURN_OK )
{
rm_pfree(AsyncCommContext, context);
- elog(WARNING, "Fail to register asynchronous connection for sending "
- "IMAlive message. %d", res);
+ elog(LOG, "failed to register asynchronous connection for sending "
+ "IMAlive message. %d", res);
+ /* Always switch if fail to register connection here. */
+ switchIMAliveSendingTarget();
--- End diff --
OK. That's fine.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq issue #1078: HAWQ-1258. segment resource manager does not swi...
Posted by paul-guo- <gi...@git.apache.org>.
Github user paul-guo- commented on the issue:
https://github.com/apache/incubator-hawq/pull/1078
+1
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #1078: HAWQ-1258. segment resource manager does ...
Posted by paul-guo- <gi...@git.apache.org>.
Github user paul-guo- commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1078#discussion_r95293276
--- Diff: src/backend/utils/misc/guc.c ---
@@ -8200,7 +8200,7 @@ static struct config_string ConfigureNamesString[] =
NULL
},
&standby_addr_host,
- "localhost", NULL, NULL
+ "none", NULL, NULL
--- End diff --
How if the user really configures the stands host as "none"? Maybe ""?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #1078: HAWQ-1258. segment resource manager does ...
Posted by paul-guo- <gi...@git.apache.org>.
Github user paul-guo- commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1078#discussion_r95515076
--- Diff: src/backend/utils/misc/guc.c ---
@@ -8200,7 +8200,7 @@ static struct config_string ConfigureNamesString[] =
NULL
},
&standby_addr_host,
- "localhost", NULL, NULL
+ "none", NULL, NULL
--- End diff --
I suspect this does not affect your code.
See the code below,
def get_all_values(self):
if 'hawq_standby_address_host' in self.hawq_dict:
if self.hawq_dict['hawq_standby_address_host'].lower() in ['none', '', 'localhost']:
del self.hawq_dict['hawq_standby_address_host']
However hawq_ctl should remove the "none" case, I think.
@radarwave Please confirm the logic.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #1078: HAWQ-1258. segment resource manager does ...
Posted by paul-guo- <gi...@git.apache.org>.
Github user paul-guo- commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1078#discussion_r95526548
--- Diff: src/backend/utils/misc/guc.c ---
@@ -8200,7 +8200,7 @@ static struct config_string ConfigureNamesString[] =
NULL
},
&standby_addr_host,
- "localhost", NULL, NULL
+ "none", NULL, NULL
--- End diff --
OK. Agree, we could start another JIRA to track the hostname "none" issue.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #1078: HAWQ-1258. segment resource manager does ...
Posted by jiny2 <gi...@git.apache.org>.
Github user jiny2 commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1078#discussion_r95294340
--- Diff: src/backend/resourcemanager/communication/rmcomm_RMSEG2RM.c ---
@@ -107,8 +107,10 @@ int sendIMAlive(int *errorcode,
if ( res != FUNC_RETURN_OK )
{
rm_pfree(AsyncCommContext, context);
- elog(WARNING, "Fail to register asynchronous connection for sending "
- "IMAlive message. %d", res);
+ elog(LOG, "failed to register asynchronous connection for sending "
+ "IMAlive message. %d", res);
+ /* Always switch if fail to register connection here. */
+ switchIMAliveSendingTarget();
--- End diff --
The interval should be 30 second, a very low frequency.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #1078: HAWQ-1258. segment resource manager does ...
Posted by jiny2 <gi...@git.apache.org>.
Github user jiny2 commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1078#discussion_r95513917
--- Diff: src/backend/utils/misc/guc.c ---
@@ -8200,7 +8200,7 @@ static struct config_string ConfigureNamesString[] =
NULL
},
&standby_addr_host,
- "localhost", NULL, NULL
+ "none", NULL, NULL
--- End diff --
So that's why we need another fix if necessary. the fix should fix script and code together. @paul-guo-
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---