You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hawq.apache.org by outofmem0ry <gi...@git.apache.org> on 2017/06/13 19:12:16 UTC
[GitHub] incubator-hawq pull request #1254: HAWQ-1373 - Added feature to reload GUC v...
GitHub user outofmem0ry opened a pull request:
https://github.com/apache/incubator-hawq/pull/1254
HAWQ-1373 - Added feature to reload GUC values using hawq reload-config
This commit introduces `hawq reload-config <object>` as a replacement of `hawq stop <object> -u|--reload`, to reload GUC values without restarting the cluster.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/outofmem0ry/incubator-hawq feature/reload-config
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hawq/pull/1254.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 #1254
----
commit ce25db4e0cda2b87c25ca44fd29807380f61dafb
Author: Shubham Sharma <sh...@gmail.com>
Date: 2017-06-13T15:02:33Z
HAWQ-1373 - Added feature to reload GUC values using hawq reload-config <object> deprecating hawq stop <object> -u
----
---
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 #1254: HAWQ-1373 - Added feature to reload GUC values u...
Posted by outofmem0ry <gi...@git.apache.org>.
Github user outofmem0ry commented on the issue:
https://github.com/apache/incubator-hawq/pull/1254
@radarwave committed the final changes, doc changes pending. Will submit a PR in docs repo.
---
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 #1254: HAWQ-1373 - Added feature to reload GUC values u...
Posted by outofmem0ry <gi...@git.apache.org>.
Github user outofmem0ry commented on the issue:
https://github.com/apache/incubator-hawq/pull/1254
@radarwave - am in the process of making final changes for this pull request, I don't see any existing test infrastructure for management utilities, am I missing something here. Can you point me to some existing test cases and how they are plugged into hawq code base.
---
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 #1254: HAWQ-1373 - Added feature to reload GUC values u...
Posted by outofmem0ry <gi...@git.apache.org>.
Github user outofmem0ry commented on the issue:
https://github.com/apache/incubator-hawq/pull/1254
Scoped out some documentation changes which are detailed below -
[Files with hawq stop cluster -u](https://github.com/apache/incubator-hawq-docs/search?utf8=%E2%9C%93&q=%22hawq+stop+cluster+-u%22&type=)
[Files with hawq stop cluster --reload](https://github.com/apache/incubator-hawq-docs/search?utf8=%E2%9C%93&q=%22hawq+stop+cluster+--reload%22&type=)
[Files with references to reload](https://github.com/apache/incubator-hawq-docs/search?p=1&q=reload+&type=&utf8=%E2%9C%93)
---
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 #1254: HAWQ-1373 - Added feature to reload GUC v...
Posted by outofmem0ry <gi...@git.apache.org>.
Github user outofmem0ry closed the pull request at:
https://github.com/apache/incubator-hawq/pull/1254
---
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 #1254: HAWQ-1373 - Added feature to reload GUC values u...
Posted by linwen <gi...@git.apache.org>.
Github user linwen commented on the issue:
https://github.com/apache/incubator-hawq/pull/1254
Shubham, I think what you've done in this PR is to add a command for hawq, which can reload GUC configs without restarting the system. Currently, this is done by this command "hawq stop cluster --reload", which is a little bit ambiguous in my opinion. So if we all agree on using "hawq reload-config" instead, the old codes related to this reloading GUC logic should be removed. @radarwave @vVineet
---
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 #1254: HAWQ-1373 - Added feature to reload GUC values u...
Posted by stanlyxiang <gi...@git.apache.org>.
Github user stanlyxiang commented on the issue:
https://github.com/apache/incubator-hawq/pull/1254
If we add "hawq reload-config", I think there is no need for "hawq stop cluster -u". 2 commands have same function make me more confused.
---
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 #1254: HAWQ-1373 - Added feature to reload GUC values u...
Posted by outofmem0ry <gi...@git.apache.org>.
Github user outofmem0ry commented on the issue:
https://github.com/apache/incubator-hawq/pull/1254
@radarwave - Fixed and incorporated above comments.
---
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 #1254: HAWQ-1373 - Added feature to reload GUC values u...
Posted by radarwave <gi...@git.apache.org>.
Github user radarwave commented on the issue:
https://github.com/apache/incubator-hawq/pull/1254
Thanks @outofmem0ry to contribute this, please check below comments.
1. I think we should keep the legacy command to do reload in case it's hard coded in some user cases, print a deprecated message should be fine.
2. Make sure all the related help messages and documents are updated.
3. Chose a proper name for this option before user start to use it, 'reload' or 'reload-config' or something else?
4. Add test for this option.
---
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 #1254: HAWQ-1373 - Added feature to reload GUC v...
Posted by outofmem0ry <gi...@git.apache.org>.
Github user outofmem0ry commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1254#discussion_r125389996
--- Diff: tools/bin/hawq ---
@@ -120,6 +120,23 @@ def main():
if second_arg not in cluster_type_list:
print STOP_HELP
sys.exit(1)
+ # Prints deprecation warning when using old syntax "hawq stop <object> -u" to reload GUC
+ if len(sub_args.split("-")) > 1:
+ if "-u" in sub_args or "--reload" in sub_args or "u" in sub_args.split("-")[1]:
+ deprecationMessage = """
+ hawq stop <object> -u is being deprecated and replaced by 'hawq reload <object>'
--- End diff --
@radarwave - I deliberately kept the messages printing different so that it is noticeable to the user, as it is a deprecation warning.
---
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 #1254: HAWQ-1373 - Added feature to reload GUC values u...
Posted by radarwave <gi...@git.apache.org>.
Github user radarwave commented on the issue:
https://github.com/apache/incubator-hawq/pull/1254
Thanks for @outofmem0ry 's contribution, I have squashed and merged this commit. Please close this PR.
Welcome to do more contributions.
---
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 #1254: HAWQ-1373 - Added feature to reload GUC values u...
Posted by radarwave <gi...@git.apache.org>.
Github user radarwave commented on the issue:
https://github.com/apache/incubator-hawq/pull/1254
@outofmem0ry
Please check the cases in below folder as example:
incubator-hawq/src/test/feature/ManagementTool
---
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 #1254: HAWQ-1373 - Added feature to reload GUC values u...
Posted by outofmem0ry <gi...@git.apache.org>.
Github user outofmem0ry commented on the issue:
https://github.com/apache/incubator-hawq/pull/1254
@linwen @stanlyxiang @radarwave - Thank you for the comments and suggestions.
@radarwave - Please see the comments inline
1. I think we should keep the legacy command to do reload in case it's hard coded in some user cases, print a deprecated message should be fine.
> The change that is committed prints a message as seen below and exits. I can change the message and let the command continue.
```
hawq stop cluster -u
To reload GUC values without restarting hawq cluster use 'hawq reload-config <object>'
```
2. Make sure all the related help messages and documents are updated.
> This is something that will go into incubator-hawq-docs and am working on scoping the change for this.
3. Chose a proper name for this option before user start to use it, 'reload' or 'reload-config' or something else?
> reload sounds good. Also, am open for any other suggestions
4. Add test for this option.
> Sure, let me see how we plug test into the codebase. Any pointers are appreciated.
---
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 #1254: HAWQ-1373 - Added feature to reload GUC v...
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/1254#discussion_r125388848
--- Diff: src/test/feature/ManagementTool/test_hawq_reload.cpp ---
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <string>
+#include <stdio.h>
+#include "lib/command.h"
+#include "lib/sql_util.h"
+#include "lib/string_util.h"
+#include "lib/hawq_config.h"
+#include "test_hawq_reload.h"
+
+#include "gtest/gtest.h"
+
+using std::string;
+using hawq::test::SQLUtility;
+using hawq::test::Command;
+
+/*
+Test case for hawq reload <object>. This test changes the value of GUC
+log_min_messages to debug. Reloads the cluster and verifies if the change
+was reloaded successfully. After the test it resets the valueof GUC to
--- End diff --
Typo 'valueof'.
---
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 #1254: HAWQ-1373 - Added feature to reload GUC values u...
Posted by radarwave <gi...@git.apache.org>.
Github user radarwave commented on the issue:
https://github.com/apache/incubator-hawq/pull/1254
LGTM +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 issue #1254: HAWQ-1373 - Added feature to reload GUC values u...
Posted by outofmem0ry <gi...@git.apache.org>.
Github user outofmem0ry commented on the issue:
https://github.com/apache/incubator-hawq/pull/1254
Thank you @linwen @radarwave . Closing this PR, will also submit a PR for documentation changes to apache/incubator-hawq-docs repo.
---
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 #1254: HAWQ-1373 - Added feature to reload GUC values u...
Posted by radarwave <gi...@git.apache.org>.
Github user radarwave commented on the issue:
https://github.com/apache/incubator-hawq/pull/1254
Need to fix:
run 'hawq reload' without any other arguments:
hawq reload
Traceback (most recent call last):
File "/usr/local/hawq/bin/hawq", line 215, in <module>
main()
File "/usr/local/hawq/bin/hawq", line 135, in main
sub_args_list.append("-u")
UnboundLocalError: local variable 'sub_args_list' referenced before assignment
Traceback (most recent call last):
File "/usr/local/hawq/bin/hawq", line 215, in <module>
main()
File "/usr/local/hawq/bin/hawq", line 135, in main
sub_args_list.append("-u")
UnboundLocalError: local variable 'sub_args_list' referenced before assignment
---
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 #1254: HAWQ-1373 - Added feature to reload GUC v...
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/1254#discussion_r125376981
--- Diff: tools/bin/hawq ---
@@ -120,6 +120,23 @@ def main():
if second_arg not in cluster_type_list:
print STOP_HELP
sys.exit(1)
+ # Prints deprecation warning when using old syntax "hawq stop <object> -u" to reload GUC
+ if len(sub_args.split("-")) > 1:
+ if "-u" in sub_args or "--reload" in sub_args or "u" in sub_args.split("-")[1]:
+ deprecationMessage = """
+ hawq stop <object> -u is being deprecated and replaced by 'hawq reload <object>'
--- End diff --
This deprecation message is not align with other messages.
Run 'hawq stop cluster -u' then you can see.
---
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 #1254: HAWQ-1373 - Added feature to reload GUC values u...
Posted by linwen <gi...@git.apache.org>.
Github user linwen commented on the issue:
https://github.com/apache/incubator-hawq/pull/1254
+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 #1254: HAWQ-1373 - Added feature to reload GUC v...
Posted by outofmem0ry <gi...@git.apache.org>.
Github user outofmem0ry commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1254#discussion_r125390001
--- Diff: src/test/feature/ManagementTool/test_hawq_reload.cpp ---
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <string>
+#include <stdio.h>
+#include "lib/command.h"
+#include "lib/sql_util.h"
+#include "lib/string_util.h"
+#include "lib/hawq_config.h"
+#include "test_hawq_reload.h"
+
+#include "gtest/gtest.h"
+
+using std::string;
+using hawq::test::SQLUtility;
+using hawq::test::Command;
+
+/*
+Test case for hawq reload <object>. This test changes the value of GUC
+log_min_messages to debug. Reloads the cluster and verifies if the change
+was reloaded successfully. After the test it resets the valueof GUC to
--- End diff --
Thank you for pointing it out, will correct it.
---
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.
---