You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hawq.apache.org by liming01 <gi...@git.apache.org> on 2015/12/23 11:32:01 UTC
[GitHub] incubator-hawq pull request: HAWQ-273. Need re-fetch snapshot at p...
GitHub user liming01 opened a pull request:
https://github.com/apache/incubator-hawq/pull/211
HAWQ-273. Need re-fetch snapshot at planning stage
Because data locality and hdfs metadata cache will fetch pg_aoseg meta data when planning, we need to re-fetch the latest snapshot.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/liming01/incubator-hawq mli/tx_alttbl_sel
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hawq/pull/211.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 #211
----
commit a5b8c0af402d120895997da2686fc207b3cae22f
Author: Ming LI <ml...@pivotal.io>
Date: 2015-12-23T10:28:29Z
HAWQ-273. Need re-fetch snapshot at planning stage because data locality and hdfs metadata cache will fetch pg_aoseg data
----
---
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: HAWQ-273. Need re-fetch snapshot at p...
Posted by liming01 <gi...@git.apache.org>.
Github user liming01 commented on the pull request:
https://github.com/apache/incubator-hawq/pull/211#issuecomment-167185478
Yes, only 2 functions:
- _SPI_prepare_plan(): you can find that before calling pg_plan_queries(), it already fetch a new snapshot. And between them, it just some simple code, which is impossible to lock. So we don't need to re-fetch one.
- exec_parse_message(): From the comment in the code, it just called for syntax check.
/**
* Even though we do not store the plan, we want to perform planning
* to know if SQL syntax is supported.
*/
---
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: HAWQ-273. Need re-fetch snapshot at p...
Posted by liming01 <gi...@git.apache.org>.
Github user liming01 closed the pull request at:
https://github.com/apache/incubator-hawq/pull/211
---
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: HAWQ-273. Need re-fetch snapshot at p...
Posted by wangzw <gi...@git.apache.org>.
Github user wangzw commented on the pull request:
https://github.com/apache/incubator-hawq/pull/211#issuecomment-167036076
It is the issue of order of locking and getting snapshot. After unlocking, snapshot become outdated and should renew.
---
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: HAWQ-273. Need re-fetch snapshot at p...
Posted by wangzw <gi...@git.apache.org>.
Github user wangzw commented on the pull request:
https://github.com/apache/incubator-hawq/pull/211#issuecomment-167034050
For PBE query, snapshot is set int PortalRunUtility(), so other changes also cause issues.
---
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: HAWQ-273. Need re-fetch snapshot at p...
Posted by liming01 <gi...@git.apache.org>.
Github user liming01 commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/211#discussion_r48400788
--- Diff: src/backend/tcop/postgres.c ---
@@ -1645,7 +1645,7 @@ exec_simple_query(const char *query_string, const char *seqServerHost, int seqSe
querytree_list = pg_analyze_and_rewrite(parsetree, query_string,
NULL, 0);
- plantree_list = pg_plan_queries(querytree_list, NULL, false, QRL_ONCE);
+ plantree_list = pg_plan_queries(querytree_list, NULL, true, QRL_ONCE);
--- End diff --
The comment is confusing, it just describes the details about implementation of this function, instead of describing the API parameter constraint. The logic of skipping snapshot for the utility statements is inside pg_plan_queries(). So it is no problem.
---
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: HAWQ-273. Need re-fetch snapshot at p...
Posted by changleicn <gi...@git.apache.org>.
Github user changleicn commented on the pull request:
https://github.com/apache/incubator-hawq/pull/211#issuecomment-167180500
looks there are still other places that call pg_plan_queries, why don't we re-fetch the snapshot?
---
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: HAWQ-273. Need re-fetch snapshot at p...
Posted by changleicn <gi...@git.apache.org>.
Github user changleicn commented on the pull request:
https://github.com/apache/incubator-hawq/pull/211#issuecomment-167195081
cool. good fix.
---
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: HAWQ-273. Need re-fetch snapshot at p...
Posted by liming01 <gi...@git.apache.org>.
Github user liming01 commented on the pull request:
https://github.com/apache/incubator-hawq/pull/211#issuecomment-167180204
In fact when executing one normal statement(SELECT/INSERT/UPDATE/DELETE), the snapshot will be fetched several times, at previous version, we fetched one snapshot before transform query, and re-fetched one at the beginning of query execution.
Your last comments above is exactly the root cause of this defect. ~_~
---
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: HAWQ-273. Need re-fetch snapshot at p...
Posted by wangzw <gi...@git.apache.org>.
Github user wangzw commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/211#discussion_r48393686
--- Diff: src/backend/tcop/postgres.c ---
@@ -1645,7 +1645,7 @@ exec_simple_query(const char *query_string, const char *seqServerHost, int seqSe
querytree_list = pg_analyze_and_rewrite(parsetree, query_string,
NULL, 0);
- plantree_list = pg_plan_queries(querytree_list, NULL, false, QRL_ONCE);
+ plantree_list = pg_plan_queries(querytree_list, NULL, true, QRL_ONCE);
--- End diff --
It's not right
```
A snapshot must be set before invoking the planner, since it
* might try to evaluate user-defined functions. But we must not set a
* snapshot if the list contains only utility statements, because some
* utility statements depend on not having frozen the snapshot yet.
```
Snapshot for plannable statement was set before, see line 1631.
---
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: HAWQ-273. Need re-fetch snapshot at p...
Posted by wangzw <gi...@git.apache.org>.
Github user wangzw commented on the pull request:
https://github.com/apache/incubator-hawq/pull/211#issuecomment-167034173
```
Because data locality and hdfs metadata cache will fetch pg_aoseg meta data when planning, we need to re-fetch the latest snapshot.
```
That's right, but all plannable statements have already got the snapshot. Did we missing something?
---
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.
---