[
https://issues.apache.org/jira/browse/IMPALA-13240?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17874078#comment-17874078
]
ASF subversion and git services commented on IMPALA-13240:
----------------------------------------------------------
Commit db0f0dadf19e8a18bdf8e7c1788e1fe2af9cb675 in impala's branch
refs/heads/master from stiga-huang
[ https://gitbox.apache.org/repos/asf?p=impala.git;h=db0f0dadf ]
IMPALA-13240: Add gerrit comments for Thrift/FlatBuffers changes
Adds gerrit comments for changes in Thrift/FlatBuffers files that could
break the communication between impalad and catalogd/statestore during
upgrade.
Basically, only new optional fields can be added in Thrift protocol. For
Flatbuffers schemas, we should only add new fields at the end of a table
definition.
Adds a new option (--revision) for critique-gerrit-review.py to specify
the revision (HEAD or a commit, branch, etc). Also adds an option
(--base-revision) to specify the base revision for comparison.
To test the script locally, prepare a virtual env with the virtualenv
package installed:
virtualenv venv
source venv/bin/activate
pip install virtualenv
Then run the script with --dryrun:
python bin/jenkins/critique-gerrit-review.py --dryrun --revision effc9df93
Limitations
- False positive in cases that add new Thrift structs with required
fields and only use those new structs in new optional fields, e.g.
effc9df93 and 72732da9d.
- Might have false positive results on reformat changes due to simple
string checks, e.g. 91d8a8f62.
- Can't check incompatible changes in FlatBuffers files. Just add
general file level comments.
We can integrate DUPCheck in the future to parse the Thrift/FlatBuffers
files to AST and compare the AST instead.
https://github.com/jwjwyoung/DUPChecker
Tests:
- Verified incompatible commits like 012996a06 and 65094a74f.
- Verified posting Gerrit comments from local env using my username.
Change-Id: Ib35fafa50bfd38631312d22464df14d426f55346
Reviewed-on: http://gerrit.cloudera.org:8080/21646
Reviewed-by: Riza Suminto <riza.sumi...@cloudera.com>
Tested-by: Quanlong Huang <huangquanl...@gmail.com>
> Add tools to detect incompatible changes in catalog and statestore service
> --------------------------------------------------------------------------
>
> Key: IMPALA-13240
> URL: https://issues.apache.org/jira/browse/IMPALA-13240
> Project: IMPALA
> Issue Type: Task
> Components: Infrastructure
> Reporter: Quanlong Huang
> Assignee: Quanlong Huang
> Priority: Critical
>
> To support upgrading only the impalads of a cluster, we need to make sure
> backward compatibility between impalad and the catalogd and statestore
> services. If there are breaking changes in Thrift/Flatbuffers definitions
> used in these services, the versions become incompatible. Note that Protobuf
> is only used in communication between impalads so can be ignored here.
> The following files need to be checked:
> * common/thrift/*.thrift
> * common/fbs/*.fbs
> Ideally changes in Thrift definitions should only add optional fields.
> Changes in Flatbuffers schema only add new fields at the end of a table
> definition.
> We need a script to verify all Gerrit changes. A precommit job running this
> script and post comments will be helpful.
> Ref:
> https://diwakergupta.github.io/thrift-missing-guide/#_defining_structs
> https://flatbuffers.dev/flatbuffers_guide_writing_schema.html
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-all-unsubscr...@impala.apache.org
For additional commands, e-mail: issues-all-h...@impala.apache.org
