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 2016/01/05 08:03:07 UTC
[GitHub] incubator-hawq pull request: HAWQ-316. Rework SSE42 implementation...
GitHub user liming01 opened a pull request:
https://github.com/apache/incubator-hawq/pull/242
HAWQ-316. Rework SSE42 implementation and runtime logic to be more similar to PostgreSQL9.5
The problem already fixed on GPDB,
https://github.com/greenplum-db/gpdb/pull/31
Here just merge the fix into hawq.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/liming01/incubator-hawq mli/sse42_crc32
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hawq/pull/242.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 #242
----
commit d5642a3dc6e177b9fcd9005718d86ba9b38d4fc4
Author: Ming LI <ml...@pivotal.io>
Date: 2016-01-05T06:58:53Z
HAWQ-316. Rework SSE42 implementation and runtime logic to be more similar to PostgreSQL9.5
----
---
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-316. Rework SSE42 implementation...
Posted by ivannovick <gi...@git.apache.org>.
Github user ivannovick commented on the pull request:
https://github.com/apache/incubator-hawq/pull/242#issuecomment-177061238
@liming01 how is this being tested? Can you summarize the test cases for this change?
---
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-316. Rework SSE42 implementation...
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/242#discussion_r48818075
--- Diff: src/bin/pg_controldata/pg_controldata.c ---
@@ -129,23 +129,24 @@ main(int argc, char *argv[])
close(fd);
/* Check the CRC. */
- crc = crc32c(crc32cInit(), &ControlFile, offsetof(ControlFileData, crc));
- crc32cFinish(crc);
+ INIT_CRC32C(crc);
--- End diff --
please fix indent 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: HAWQ-316. Rework SSE42 implementation...
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/242#discussion_r48818051
--- Diff: src/bin/gpupgrade/gpmodcatversion.c ---
@@ -224,14 +225,16 @@ main(int argc, char *argv[])
ControlFile.catalog_version_no = tover;
/* recalcualte the CRC. */
- crc = crc32c(crc32cInit(), &ControlFile, offsetof(ControlFileData, crc));
- crc32cFinish(crc);
+ INIT_CRC32C(crc);
+ COMP_CRC32C(crc, &ToControlFile, offsetof(ControlFileData, crc));
+ FIN_CRC32C(crc);
+
--- End diff --
please fix indent 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: HAWQ-316. Rework SSE42 implementation...
Posted by liming01 <gi...@git.apache.org>.
Github user liming01 commented on the pull request:
https://github.com/apache/incubator-hawq/pull/242#issuecomment-180169068
1) Not specific test case for this changes.
2) By default, although HAWQ disable the checksum verification for interconnect module, the catalog stored in local disk will use 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.
---
[GitHub] incubator-hawq pull request: HAWQ-316. Rework SSE42 implementation...
Posted by linwen <gi...@git.apache.org>.
Github user linwen commented on the pull request:
https://github.com/apache/incubator-hawq/pull/242#issuecomment-185579185
+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: HAWQ-316. Rework SSE42 implementation...
Posted by cwelton <gi...@git.apache.org>.
Github user cwelton commented on the pull request:
https://github.com/apache/incubator-hawq/pull/242#issuecomment-185327575
This PR has now been open for over 30 days.
I see that HAWQ-316 has been marked resolved, did you simply forget to close the PR?
---
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-316. Rework SSE42 implementation...
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/242#discussion_r48817881
--- Diff: src/backend/access/transam/xlog.c ---
@@ -3832,7 +3833,8 @@ RecordIsValid(XLogRecord *record, XLogRecPtr recptr, int emode)
*/
/* First the rmgr data */
- crc = crc32c(crc32cInit(), XLogRecGetData(record), len);
+ INIT_CRC32C(crc);
--- End diff --
please fix indent 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: HAWQ-316. Rework SSE42 implementation...
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/242#discussion_r48817999
--- Diff: src/bin/gpupgrade/gpmodcatversion.c ---
@@ -174,19 +174,20 @@ main(int argc, char *argv[])
}
/* Check the CRC. */
- crc = crc32c(crc32cInit(), &ControlFile, offsetof(ControlFileData, crc));
- crc32cFinish(crc);
+ INIT_CRC32C(crc);
--- End diff --
please fix indent 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: HAWQ-316. Rework SSE42 implementation...
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/242#discussion_r48817889
--- Diff: src/backend/access/transam/xlog.c ---
@@ -4914,10 +4916,11 @@ WriteControlFile(void)
StrNCpy(ControlFile->lc_ctype, localeptr, LOCALE_NAME_BUFLEN);
/* Contents are protected with a CRC */
- ControlFile->crc = crc32c(crc32cInit(),
+ INIT_CRC32C(ControlFile->crc);
--- End diff --
please fix indent 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: HAWQ-316. Rework SSE42 implementation...
Posted by cwelton <gi...@git.apache.org>.
Github user cwelton commented on the pull request:
https://github.com/apache/incubator-hawq/pull/242#issuecomment-176997975
What will it take to get this moving again? Can we merge it or close it out?
---
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-316. Rework SSE42 implementation...
Posted by changleicn <gi...@git.apache.org>.
Github user changleicn commented on the pull request:
https://github.com/apache/incubator-hawq/pull/242#issuecomment-185571957
looks good!
---
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-316. Rework SSE42 implementation...
Posted by liming01 <gi...@git.apache.org>.
Github user liming01 commented on the pull request:
https://github.com/apache/incubator-hawq/pull/242#issuecomment-185507024
Not yet, I haven't gotten approval to merge it.
I will merge it ASAP when getting approval. 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: HAWQ-316. Rework SSE42 implementation...
Posted by liming01 <gi...@git.apache.org>.
Github user liming01 closed the pull request at:
https://github.com/apache/incubator-hawq/pull/242
---
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-316. Rework SSE42 implementation...
Posted by wangzw <gi...@git.apache.org>.
Github user wangzw commented on the pull request:
https://github.com/apache/incubator-hawq/pull/242#issuecomment-168922445
The patch is generally OK but I have some concerns.
1) Do we have test case for CRC calculation?
2) By default the checksum verify for interconnect module is disable, but we do not have test case for CRC enable interconnect. It make code change unsafe.
---
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.
---