[Impala-ASF-CR] IMPALA-4740: Add option to use hdfsPread() for HDFS hedged reads
Huaisi Xu has posted comments on this change. Change subject: IMPALA-4740: Add option to use hdfsPread() for HDFS hedged reads .. Patch Set 2: > > The HDFS-5776 JIRA mentions that it's implemented for the pread() > > code path, but it's not that clear. If you look at the code it's > > very clear that it only kicks in on the pread() code path: > > > > https://github.com/apache/hadoop/search?utf8=%E2%9C%93&q=isHedgedReadsEnabled > > I may be wrong again (as always), hdfspread is not pread. from > https://github.com/apache/hadoop/blob/f67237cbe7bc48a1b9088e990800b37529f1db2a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/hdfs.c#L1426, > you can see hdfspread() invokes read(), and hdfsread() calls read() > as well, and in turn read() calls pread() as you can see in the > link you just quoted. So this new gflag is essentially ignored? > What did I miss here? I missed. it calls a different signature. hdfspread calls read(long position, byte[] buffer, int offset, int length). thanks for pointing it out.. -- To view, visit http://gerrit.cloudera.org:8080/5635 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iecc8b12aa20cbfe08f4ef6a08a191e49709d9525 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4740: Add option to use hdfsPread() for HDFS hedged reads
Huaisi Xu has posted comments on this change. Change subject: IMPALA-4740: Add option to use hdfsPread() for HDFS hedged reads .. Patch Set 2: > The HDFS-5776 JIRA mentions that it's implemented for the pread() > code path, but it's not that clear. If you look at the code it's > very clear that it only kicks in on the pread() code path: > > https://github.com/apache/hadoop/search?utf8=%E2%9C%93&q=isHedgedReadsEnabled I may be wrong again (as always), hdfspread is not pread. from https://github.com/apache/hadoop/blob/f67237cbe7bc48a1b9088e990800b37529f1db2a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/hdfs.c#L1426, you can see hdfspread() invokes read(), and hdfsread() calls read() as well, and in turn read() calls pread() as you can see in the link you just quoted. So this new gflag is essentially ignored? What did I miss here? -- To view, visit http://gerrit.cloudera.org:8080/5635 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iecc8b12aa20cbfe08f4ef6a08a191e49709d9525 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4740: Add option to use hdfsPread() for HDFS hedged reads
Huaisi Xu has posted comments on this change. Change subject: IMPALA-4740: Add option to use hdfsPread() for HDFS hedged reads .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/5635/2/be/src/runtime/disk-io-mgr-scan-range.cc File be/src/runtime/disk-io-mgr-scan-range.cc: PS2, Line 408: hdfsPread could you point to the reference that we need hdfsPread in order to make use of hdfs hedged read? from HDFS-5776 looks like it is automatically enabled for Impala if the HDFS client is configured so? -- To view, visit http://gerrit.cloudera.org:8080/5635 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iecc8b12aa20cbfe08f4ef6a08a191e49709d9525 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Huaisi Xu has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/4867/1/be/src/catalog/catalog.cc File be/src/catalog/catalog.cc: PS1, Line 78: catalog_class_, catalog_ctor_, : load_in_background, num_metadata_loading_threads, sentry_config, : FlagToTLogLevel(FLAGS_v), FlagToTLogLevel(FLAGS_non_impala_java_vlog), : auth_to_local, principal > How about passing a TBackendConfig directly here. Basically make the constr I remember you asked for a generic (config) for all jvms, since some of these are catalog specific, I think the choice he has is to have a jvm config with separate sub field for different services. http://gerrit.cloudera.org:8080/#/c/4867/1/common/thrift/Types.thrift File common/thrift/Types.thrift: PS1, Line 233: TBackendConfig This is not a backend config. it is more like a jvm config.. or config from backend? I think technically there is no "backend" outside of impalad. e.g. you cannot say statestore and catalog are backends. Just my two cents as this already becomes overly complicated. -- To view, visit http://gerrit.cloudera.org:8080/4867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33684725a61eabc67237503e61178305d37d3cb5 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce single-table consistency in query analysis.
Huaisi Xu has uploaded a new patch set (#17). Change subject: IMPALA-1702: Enforce single-table consistency in query analysis. .. IMPALA-1702: Enforce single-table consistency in query analysis. Catalogd managed table id generation causes problems when new updates arrive at frontend while queries are being analyzed: references to the same table may suddenly refer to a different version, and different tables may share the same table id. This causes problems when one table overrides another one in the thrift descriptor table sent to backend. This commit removes the table id from the catalog Table object; instead frontend assigns a unique id to each table in DescriptorTable.toThrift(). It also implements a referencedTables_ cache in Analyzer::globalState_ so that calling Analyzer::getTable() on the same table returns the same reference for the same query. Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 --- M common/thrift/CatalogObjects.thrift M common/thrift/Descriptors.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java D fe/src/main/java/org/apache/impala/catalog/TableId.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/common/Id.java M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/JoinTableId.java M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java 27 files changed, 204 insertions(+), 219 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4349/17 -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 17 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1702: Enforce single-table consistency in query analysis.
Hello Marcel Kornacker, Bharath Vissapragada, Alex Behm, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4349 to look at the new patch set (#17). Change subject: IMPALA-1702: Enforce single-table consistency in query analysis. .. IMPALA-1702: Enforce single-table consistency in query analysis. Catalogd managed table id generation causes problems when new updates arrive at frontend while queries are being analyzed: references to the same table may suddenly refer to a different version, and different tables may share the same table id. This causes problems when one table overrides another one in the thrift descriptor table sent to backend. This commit removes the table id from the catalog Table object; instead frontend assigns a unique id to each table in DescriptorTable.toThrift(). It also implements a referencedTables_ cache in Analyzer::globalState_ so that calling Analyzer::getTable() on the same table returns the same reference for the same query. Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 --- M common/thrift/CatalogObjects.thrift M common/thrift/Descriptors.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java D fe/src/main/java/org/apache/impala/catalog/TableId.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/common/Id.java M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/JoinTableId.java M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java 27 files changed, 204 insertions(+), 219 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4349/17 -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 17 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1702: Enforce single-table consistency in query analysis.
Huaisi Xu has posted comments on this change.
Change subject: IMPALA-1702: Enforce single-table consistency in query analysis.
..
Patch Set 16:
(10 comments)
Thanks Marcel.
http://gerrit.cloudera.org:8080/#/c/4349/15/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
Line 294: // The Impalad Catalog has the latest tables from the statestore.
In order to use the
> add todo that we need to investigate whether to do this for other catalog o
Done
http://gerrit.cloudera.org:8080/#/c/4349/15/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
File fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java:
Line 43: * them unique ids.
> "may be null" is a better phrasing.
Done
Line 44: */
> "Set in QueryStmt.analyze()."
Done
Line 45: public class DescriptorTable {
> remove last sentence, that goes without saying.
Done
Line 179: // inline view of a non-constant select has a non-materialized
tuple descriptor
> "checking that"
Done
Line 180: // in the descriptor table just for type checking, which we
need to skip
> "same Table instance" is even more specific (because we're talking about ob
Done
Line 202: for (SlotDescriptor slotD: tupleDesc.getMaterializedSlots()) {
> single line
Done
http://gerrit.cloudera.org:8080/#/c/4349/15/fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
File fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java:
Line 201: public void materializeSlots() {
> single line
done. but not sure if this is what you meant.
http://gerrit.cloudera.org:8080/#/c/4349/15/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java:
Line 438: if (descTbl.isSetTupleDescriptors()) {
> precede w/ blank line
Done
Line 447: if (execRequest.isSetFragments() &&
!execRequest.fragments.isEmpty()) {
> same here
Done
--
To view, visit http://gerrit.cloudera.org:8080/4349
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 16
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Marcel Kornacker
Gerrit-Reviewer: Tim Armstrong
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce single-table consistency in query analysis.
Huaisi Xu has posted comments on this change. Change subject: IMPALA-1702: Enforce single-table consistency in query analysis. .. Patch Set 16: solved conflict first. addressing comment next -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 16 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1702: Enforce single-table consistency in query analysis.
Huaisi Xu has uploaded a new patch set (#16). Change subject: IMPALA-1702: Enforce single-table consistency in query analysis. .. IMPALA-1702: Enforce single-table consistency in query analysis. Catalogd managed table id generation causes problems when new updates arrive at frontend while queries are being analyzed: references to the same table may suddenly refer to a different version, and different tables may share the same table id. This causes problems when one table overrides another one in the thrift descriptor table sent to backend. This commit removes the table id from the catalog Table object; instead frontend assigns a unique id to each table in DescriptorTable.toThrift(). It also implements a referencedTables_ cache in Analyzer::globalState_ so that calling Analyzer::getTable() on the same table returns the same reference for the same query. Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 --- M common/thrift/CatalogObjects.thrift M common/thrift/Descriptors.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java D fe/src/main/java/org/apache/impala/catalog/TableId.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/common/Id.java M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/JoinTableId.java M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java 27 files changed, 199 insertions(+), 213 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4349/16 -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 16 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1702: Enforce single-table consistency in query analysis.
Hello Marcel Kornacker, Bharath Vissapragada, Alex Behm, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4349 to look at the new patch set (#16). Change subject: IMPALA-1702: Enforce single-table consistency in query analysis. .. IMPALA-1702: Enforce single-table consistency in query analysis. Catalogd managed table id generation causes problems when new updates arrive at frontend while queries are being analyzed: references to the same table may suddenly refer to a different version, and different tables may share the same table id. This causes problems when one table overrides another one in the thrift descriptor table sent to backend. This commit removes the table id from the catalog Table object; instead frontend assigns a unique id to each table in DescriptorTable.toThrift(). It also implements a referencedTables_ cache in Analyzer::globalState_ so that calling Analyzer::getTable() on the same table returns the same reference for the same query. Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 --- M common/thrift/CatalogObjects.thrift M common/thrift/Descriptors.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java D fe/src/main/java/org/apache/impala/catalog/TableId.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/common/Id.java M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/JoinTableId.java M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java 27 files changed, 199 insertions(+), 213 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4349/16 -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 16 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Huaisi Xu has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: PS1, Line 169: MAX_INCREMENTAL_STATS_SIZE_BYTES > - Actually implementing this way seems to be little confusing to me. Reason I think he can try to use runtimeEnv as well. or have this in catalog not in a table. -- To view, visit http://gerrit.cloudera.org:8080/4772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d189ffc1c56e7a4e27e6233e9119250a2395a19 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Huaisi Xu has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: actually this can this be checked when we load the table, and this should be evicted once we load the incremental stats. take a look at hdfsPartition.java::getFilteredHmsParameters(), it returns a new map. This makes no sense since we will never return the stats in the life time of catalog unless some commands lower the size. I have not looked at the whole process but we should not keep them in memory as possible as we can(not breaking other things). Suggest you manually filter the keys in place. -- To view, visit http://gerrit.cloudera.org:8080/4772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d189ffc1c56e7a4e27e6233e9119250a2395a19 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Huaisi Xu has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/4772/1/be/src/catalog/catalog.cc File be/src/catalog/catalog.cc: PS1, Line 33: inc_stat_max_size > thanks. I will add the unit in option description. (in DEFINE_uint64(...) Sorry. I meant from the name we should know which unit it is. http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: PS1, Line 176: 0 > Actually, HdfsTable.MAX_INCREMENTAL_STATS_SIZE_BYTES is set in CatalogServi that is set by you right? I think a static member should not be set like this anyway. http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/common/RuntimeEnv.java File fe/src/main/java/org/apache/impala/common/RuntimeEnv.java: PS1, Line 71: incStatMaxSize > Sure. :) Do you recommend me to update computeLineage() as well? do not care about the rest.. :-) -- To view, visit http://gerrit.cloudera.org:8080/4772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d189ffc1c56e7a4e27e6233e9119250a2395a19 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Huaisi Xu has posted comments on this change. Change subject: IMPALA-3552: make incremental stats max serialized size configurable .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: PS1, Line 176: 0 > This should not be a static variable since no one outside this class is usi sorry I meant should not be a public static.. -- To view, visit http://gerrit.cloudera.org:8080/4772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d189ffc1c56e7a4e27e6233e9119250a2395a19 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Yonghyun Hwang Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Yonghyun Hwang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3552: make incremental stats max serialized size configurable
Huaisi Xu has posted comments on this change.
Change subject: IMPALA-3552: make incremental stats max serialized size
configurable
..
Patch Set 1:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/4772/1/be/src/catalog/catalog.cc
File be/src/catalog/catalog.cc:
PS1, Line 33: inc_stat_max_size
could you be explicit on the unit?
http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
Line 164: SentryConfig sentryConfig, TUniqueId catalogServiceId, String
kerberosPrincipal, long incStatMaxSize) {
long line
http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:
PS1, Line 176: 0
This should not be a static variable since no one outside this class is using
this anymore.
http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
File fe/src/main/java/org/apache/impala/common/RuntimeEnv.java:
PS1, Line 71: incStatMaxSize
getIncStatMaxSize
http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/main/java/org/apache/impala/service/JniCatalog.java
File fe/src/main/java/org/apache/impala/service/JniCatalog.java:
Line 84: boolean allowAuthToLocal, String kerberosPrincipal, long
incStatMaxSize) throws InternalException {
long line
Line 101: numMetadataLoadingThreads, sentryConfig, getServiceId(),
kerberosPrincipal, incStatMaxSize);
long
http://gerrit.cloudera.org:8080/#/c/4772/1/fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
File fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java:
Line 34: super(loadInBackground, numLoadingThreads, sentryConfig,
catalogServiceId, null, 200*1024*1024);
long
--
To view, visit http://gerrit.cloudera.org:8080/4772
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d189ffc1c56e7a4e27e6233e9119250a2395a19
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Yonghyun Hwang
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce single-table consistency in query analysis.
Huaisi Xu has posted comments on this change. Change subject: IMPALA-1702: Enforce single-table consistency in query analysis. .. Patch Set 15: > The concern that Marcel raised is also related to > https://issues.cloudera.org/browse/IMPALA-3568 > and I agree with Alex that this JIRA may not be the best vehicle to > address it. I am ok with keeping the Table Cache and documenting > the expected behavior. thanks -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 15 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1702: Enforce single-table consistency in query analysis.
Huaisi Xu has posted comments on this change. Change subject: IMPALA-1702: Enforce single-table consistency in query analysis. .. Patch Set 15: > (3 comments) Should I do the same for other catalog object as well? e.g. data source? -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 15 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1702: Enforce single-table consistency in query analysis.
Hello Bharath Vissapragada, Alex Behm, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4349 to look at the new patch set (#15). Change subject: IMPALA-1702: Enforce single-table consistency in query analysis. .. IMPALA-1702: Enforce single-table consistency in query analysis. Catalogd managed table id generation causes problems when new updates arrive at frontend while queries are being analyzed: references to the same table may suddenly refer to a different version, and different tables may share the same table id. This causes problems when one table overrides another one in the thrift descriptor table sent to backend. This commit removes the table id from the catalog Table object; instead frontend assigns a unique id to each table in DescriptorTable.toThrift(). It also implements a referencedTables_ cache in Analyzer::globalState_ so that calling Analyzer::getTable() on the same table returns the same reference for the same query. Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 --- M common/thrift/CatalogObjects.thrift M common/thrift/Descriptors.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java D fe/src/main/java/org/apache/impala/catalog/TableId.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/common/Id.java M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/JoinTableId.java M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java 27 files changed, 198 insertions(+), 214 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4349/15 -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 15 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1702: Enforce single-table consistency in query analysis.
Huaisi Xu has posted comments on this change. Change subject: IMPALA-1702: Enforce single-table consistency in query analysis. .. Patch Set 14: (4 comments) thanks http://gerrit.cloudera.org:8080/#/c/4349/14/common/thrift/CatalogObjects.thrift File common/thrift/CatalogObjects.thrift: Line 366 > there's a TTableDescriptor.table_id which deserves a comment (clarify that Done http://gerrit.cloudera.org:8080/#/c/4349/14/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: Line 294: // same version of table in a single query, we cache all referenced tables here. > "of a table" Done. Yes. but I think we definitely should not cache Db at this time since we maybe expecting a "complete" table arriving. For others, I am not sure about the others. maybe we should consider those in another Jira? Line 2284:* the table has not yet been loaded in the local catalog cache. > update comment: effect on referencedTables_ Done http://gerrit.cloudera.org:8080/#/c/4349/14/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java File fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java: PS14, Line 44: Table targetTable_ > will update comment. basically this is used to reserve id 0 to table sink b Done, but may need reword? -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 14 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce single-table consistency in query analysis.
Huaisi Xu has posted comments on this change. Change subject: IMPALA-1702: Enforce single-table consistency in query analysis. .. Patch Set 14: (1 comment) http://gerrit.cloudera.org:8080/#/c/4349/14/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java File fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java: PS14, Line 44: Table targetTable_ will update comment. basically this is used to reserve id 0 to table sink because we have a separate logic in backend handling table sinks. this is also used in partition pruning. -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 14 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce single-table consistency in query analysis.
Huaisi Xu has posted comments on this change. Change subject: IMPALA-1702: Enforce single-table consistency in query analysis. .. Patch Set 14: Code-Review+1 (2 comments) carry Alex +1 http://gerrit.cloudera.org:8080/#/c/4349/14/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java: PS14, Line 429: extra space will remove in next patch PS14, Line 439: and here -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 14 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce single-table consistency in query analysis.
Huaisi Xu has posted comments on this change.
Change subject: IMPALA-1702: Enforce single-table consistency in query analysis.
..
Patch Set 14:
(3 comments)
thanks
http://gerrit.cloudera.org:8080/#/c/4349/13/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java:
PS13, Line 425: HashSet seenTableIds = Sets.newHashSet();
:
> will clean this.
Done
Line 439: for (TTupleDescriptor tupleDesc : descTbl.tupleDescriptors) {
> if (execRequest.isSetTupleDescriptors()) {
Done
Line 452: if (!seenTableIds.contains(tableSink.target_table_id)
> empty check not needed
Done
--
To view, visit http://gerrit.cloudera.org:8080/4349
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 14
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Tim Armstrong
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce single-table consistency in query analysis.
Hello Bharath Vissapragada, Alex Behm, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4349 to look at the new patch set (#14). Change subject: IMPALA-1702: Enforce single-table consistency in query analysis. .. IMPALA-1702: Enforce single-table consistency in query analysis. Catalogd managed table id generation causes problems when new updates arrive at frontend while queries are being analyzed: references to the same table may suddenly refer to a different version, and different tables may share the same table id. This causes problems when one table overrides another one in the thrift descriptor table sent to backend. This commit removes the table id from the catalog Table object; instead frontend assigns a unique id to each table in DescriptorTable.toThrift(). It also implements a referencedTables_ cache in Analyzer::globalState_ so that calling Analyzer::getTable() on the same table returns the same reference for the same query. Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 --- M common/thrift/CatalogObjects.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java D fe/src/main/java/org/apache/impala/catalog/TableId.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/common/Id.java M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/JoinTableId.java M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java 26 files changed, 193 insertions(+), 214 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4349/14 -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 14 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1702: Enforce single-table consistency in query analysis.
Huaisi Xu has posted comments on this change.
Change subject: IMPALA-1702: Enforce single-table consistency in query analysis.
..
Patch Set 13:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4349/13/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java:
PS13, Line 425: if (execRequest.isSetFragments())
:
will clean this.
Line 439: }
if (execRequest.isSetTupleDescriptors()) {
...
}
--
To view, visit http://gerrit.cloudera.org:8080/4349
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 13
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Tim Armstrong
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce single-table consistency in query analysis.
Huaisi Xu has posted comments on this change.
Change subject: IMPALA-1702: Enforce single-table consistency in query analysis.
..
Patch Set 12:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/4349/12//COMMIT_MSG
Commit Message:
Line 7: IMPALA-1702: Enforce table level consistency
> Enforce single-table consistency in query analysis.
Done
Line 10: new update arrives at frontend while queries are being
> new updates arrive at the frontend
Done
Line 12: to a different version, and different table may share the
> different tables
Done
Line 14: another in the backend table map.
> another one in the thrift descriptor table sent to the backend.
Done
Line 16: This commit removes table id from the Table object; instead
> removes the table if from the catalog Table object
Done
Line 17: frontend assigns a unique id to each table just before
> the frontend assigns a unique id to each table in DescriptorTable.toThrift(
Done
http://gerrit.cloudera.org:8080/#/c/4349/12/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java:
Line 417:* Validates that all tables in the descriptor table of 'request'
have a unique id.
> Validates that all tables in the descriptor table of 'request' have a uniqu
Done
Line 422: if (execRequest.isSetDesc_tbl()) {
> I will merge that to line 420 next patch..
Done
Line 424: if (descTbl.isSetTableDescriptors()) {
> and merge this
Done
Line 434: for (TTupleDescriptor tupleDesc: descTbl.tupleDescriptors) {
> also check the table sink's id (if there is a sink)
Done
--
To view, visit http://gerrit.cloudera.org:8080/4349
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 12
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Tim Armstrong
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce single-table consistency in query analysis.
Hello Bharath Vissapragada, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4349 to look at the new patch set (#13). Change subject: IMPALA-1702: Enforce single-table consistency in query analysis. .. IMPALA-1702: Enforce single-table consistency in query analysis. Catalogd managed table id generation causes problems when new updates arrive at frontend while queries are being analyzed: references to the same table may suddenly refer to a different version, and different tables may share the same table id. This causes problems when one table overrides another one in the thrift descriptor table sent to backend. This commit removes the table id from the catalog Table object; instead frontend assigns a unique id to each table in DescriptorTable.toThrift(). It also implements a referecedTables_ cache in Analyzer::globalState_ so that calling Analyzer::getTable() on the same table returns the same reference for the same query. Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 --- M common/thrift/CatalogObjects.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java D fe/src/main/java/org/apache/impala/catalog/TableId.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/common/Id.java M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/JoinTableId.java M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java 26 files changed, 194 insertions(+), 214 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4349/13 -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 13 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency
Huaisi Xu has posted comments on this change.
Change subject: IMPALA-1702: Enforce table level consistency
..
Patch Set 12:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4349/12/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java:
Line 422: if (execRequest.isSetDesc_tbl()) {
I will merge that to line 420 next patch..
Line 424: if (descTbl.isSetTableDescriptors()) {
and merge this
--
To view, visit http://gerrit.cloudera.org:8080/4349
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 12
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Tim Armstrong
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency
Huaisi Xu has posted comments on this change. Change subject: IMPALA-1702: Enforce table level consistency .. Patch Set 11: (8 comments) thanks! http://gerrit.cloudera.org:8080/#/c/4349/11//COMMIT_MSG Commit Message: Line 11: analyzing: reference to the same table may refer to a > analyzed Done Line 12: different version, and different table may share same > the same Done Line 16: This commit removes table Id from Table object; instead > the Table Done PS11, Line 17: it > execRequest Done PS11, Line 21: references > reference Done http://gerrit.cloudera.org:8080/#/c/4349/11/fe/src/main/java/org/apache/impala/catalog/View.java File fe/src/main/java/org/apache/impala/catalog/View.java: PS11, Line 92: testig > not your change, but please fix the typo. Done. http://gerrit.cloudera.org:8080/#/c/4349/11/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java: Line 426: HashSet seenTableId = Sets.newHashSet(); > seenTableIds Done Line 435: } > Sorry to keep adding to this, but let's also loop through all tuple descrip Done -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 11 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency
Hello Bharath Vissapragada, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4349 to look at the new patch set (#12). Change subject: IMPALA-1702: Enforce table level consistency .. IMPALA-1702: Enforce table level consistency Catalogd managed table id generation causes problems when new update arrives at frontend while queries are being analyzed: references to the same table may suddenly refer to a different version, and different table may share the same table id. This causes problems when one table overrides another in the backend table map. This commit removes table id from the Table object; instead frontend assigns a unique id to each table just before execRequest is sent to backend. It also implements a referecedTables_ cache in Analyzer::globalState_ so that calling Analyzer::getTable() on the same table returns the same reference for the same query. Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 --- M common/thrift/CatalogObjects.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java D fe/src/main/java/org/apache/impala/catalog/TableId.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/common/Id.java M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/JoinTableId.java M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java 25 files changed, 174 insertions(+), 213 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4349/12 -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 12 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency
Huaisi Xu has posted comments on this change. Change subject: IMPALA-1702: Enforce table level consistency .. Patch Set 11: (5 comments) http://gerrit.cloudera.org:8080/#/c/4349/11//COMMIT_MSG Commit Message: Line 11: analyzing: reference to the same table may refer to a analyzed Line 12: different version, and different table may share same the same Line 16: This commit removes table Id from Table object; instead the Table PS11, Line 17: it execRequest PS11, Line 21: references reference -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 11 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency
Huaisi Xu has posted comments on this change.
Change subject: IMPALA-1702: Enforce table level consistency
..
Patch Set 10:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/4349/10/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
File fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java:
Line 155:* Connect tupleDescriptors to tableDescriptors with unique table
ids and get
> Returns the thrift representation of this DescriptorTable. Assigns unique i
Done
http://gerrit.cloudera.org:8080/#/c/4349/10/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java:
Line 418:* Our single query table consistency guarantee:
> Way too elaborate this comment. just say:
Done
Line 422:* 2. All tables reference to the same version of table in
Analyzer::getTable().
> Remove this block
Done
Line 426: private void validateTableConsistency(TExecRequest request) {
> validateTableIds
Done
Line 433: for (TTableDescriptor tableDesc: descTbl.tableDescriptors) {
> This code seems pretty elaborate for checking unique ids. Wouldn't it be su
Done. but that way we do not know the old table's name.
Line 437: throw new IllegalStateException("Failed to verify table
consistency for" +
> We're not validating table consistency. Error report should be consistent w
Done
--
To view, visit http://gerrit.cloudera.org:8080/4349
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Tim Armstrong
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency
Huaisi Xu has uploaded a new patch set (#11). Change subject: IMPALA-1702: Enforce table level consistency .. IMPALA-1702: Enforce table level consistency Catalogd managed table id generation causes problems when new update arrives at frontend while queries are being analyzing: reference to the same table may refer to a different version, and different table may share same table id. This causes problems when one table override another in the backend table map. This commit removes table Id from Table object; instead frontend assigns a unique Id to each table just before it is sent to backend. It also implements a referecedTables_ cache in Analyzer::globalState_ so that calling Analyzer::getTable() on the same table returns the same references for the same query. Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 --- M common/thrift/CatalogObjects.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java D fe/src/main/java/org/apache/impala/catalog/TableId.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/common/Id.java M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/JoinTableId.java M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java 25 files changed, 168 insertions(+), 212 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4349/11 -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 11 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency
Huaisi Xu has posted comments on this change. Change subject: IMPALA-1702: Enforce table level consistency .. Patch Set 10: (6 comments) http://gerrit.cloudera.org:8080/#/c/4349/9//COMMIT_MSG Commit Message: PS9, Line 7: Enforce table level consistency > IMO, this is a little misleading as its sounds similar to consistency guara I think this is just a high level description. It hides implementation details. This is the goal we are trying to achieve. and it contains two things. http://gerrit.cloudera.org:8080/#/c/4349/9/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: Line 295: private final HashMap referencedTables_ = Maps.newHashMap(); > Thanks for the explanation Alex. Makes sense to me. Although this is not strictly necessary, this is more of completing the circle. http://gerrit.cloudera.org:8080/#/c/4349/9/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java File fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java: PS9, Line 187: : table.load(true, client.getHiveClient(), msTbl); : insertStmt_.setTargetTable(table); > update this as per new design. Done http://gerrit.cloudera.org:8080/#/c/4349/9/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java File fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java: PS9, Line 154: /** :* Connect tupleDescriptors to tableDescriptors with unique table ids and get :* this DescriptorTable ready to be sent to backend. :*/ > How about we prefix this comment with a brief description of what this meth Done Line 164: > Sorry, I see what you mean now. Yes, that would work as well, but we'd need This is only for table validation so same table references have the same instance. If this is for validation purposes then it makes more sense to isolate this from anything we used in the code. http://gerrit.cloudera.org:8080/#/c/4349/9/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java: Line 37: import org.apache.kudu.client.KuduClient; > Move it below to org.apache.impala group. Done -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency
Huaisi Xu has uploaded a new patch set (#10). Change subject: IMPALA-1702: Enforce table level consistency .. IMPALA-1702: Enforce table level consistency Catalogd managed table id generation causes problems when new update arrives at frontend while queries are being analyzing: reference to the same table may refer to a different version, and different table may share same table id. This causes problems when one table override another in the backend table map. This commit removes table Id from Table object; instead frontend assigns a unique Id to each table just before it is sent to backend. It also implements a referecedTables_ cache in Analyzer::globalState_ so that calling Analyzer::getTable() on the same table returns the same references for the same query. Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 --- M common/thrift/CatalogObjects.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java D fe/src/main/java/org/apache/impala/catalog/TableId.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/common/Id.java M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/JoinTableId.java M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java 25 files changed, 176 insertions(+), 212 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4349/10 -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency accross service
Huaisi Xu has posted comments on this change.
Change subject: IMPALA-1702: Enforce table level consistency accross service
..
Patch Set 9:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/4349/8/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
File fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java:
Line 154: /**
> Use /** */ style comment
Done
Line 156:* Verify table level consistency in the same query by checking
references to the same
> are the same instance -> refer to the same table instance.
Done
Line 162: HashMap tableIdMap = Maps.newHashMap();
> also add targetTable_ to this map if needed
Done
Line 173: if (!tupleDesc.isMaterialized()) continue;
> "old" seems a little odd here, maybe something like checkTable?
Done
http://gerrit.cloudera.org:8080/#/c/4349/8/fe/src/main/java/org/apache/impala/analysis/TableName.java
File fe/src/main/java/org/apache/impala/analysis/TableName.java:
Line 49: public String getTbl() { return tbl_; }
> Table.getTableName() does the same thing
Done
http://gerrit.cloudera.org:8080/#/c/4349/8/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java:
Line 421:*
> DescriptorTable.toThrift() already does this, so we don't need to check it
Done
--
To view, visit http://gerrit.cloudera.org:8080/4349
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Tim Armstrong
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency accross service
Huaisi Xu has uploaded a new patch set (#9). Change subject: IMPALA-1702: Enforce table level consistency accross service .. IMPALA-1702: Enforce table level consistency accross service Problems: 1. CatalogServiceCatalog::reset() resets table ID, so same tables can have different ID and different tables can have same table ID before/after this call. Backend could overwrites each table in an 1:1 table ID to table descriptor map. 2. 1 can happen because frontend gets table directly from catalog cache, which is updated concurrently with query analyzing. so Analyzer::getTable() could return different references for the same table when analyzing the same query. This commit removes table Id from Table object; instead frontend assigns a unique Id to each table just before it is sent to backend. It also implements a referecedTables_ cache in Analyzer::globalState_ so that calling Analyzer::getTable() on the same table returns the same references for the same query. Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 --- M common/thrift/CatalogObjects.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java D fe/src/main/java/org/apache/impala/catalog/TableId.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/common/Id.java M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/JoinTableId.java M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java 25 files changed, 178 insertions(+), 209 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4349/9 -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 9 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency accross service
Huaisi Xu has uploaded a new patch set (#8). Change subject: IMPALA-1702: Enforce table level consistency accross service .. IMPALA-1702: Enforce table level consistency accross service Problems: 1. CatalogServiceCatalog::reset() resets table ID, so same tables can have different ID and different tables can have same table ID before/after this call. Backend could overwrites each table in an 1:1 table ID to table descriptor map. 2. 1 can happen because frontend gets table directly from catalog cache, which is updated concurrently with query analyzing. so Analyzer::getTable() could return different references for the same table when analyzing the same query. This commit removes table Id from Table object; instead frontend assigns a unique Id to each table just before it is sent to backend. It also implements a referecedTables_ cache in Analyzer::globalState_ so that calling Analyzer::getTable() on the same table returns the same references for the same query. Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 --- M common/thrift/CatalogObjects.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/TableName.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java D fe/src/main/java/org/apache/impala/catalog/TableId.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/common/Id.java M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/JoinTableId.java M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java 26 files changed, 178 insertions(+), 209 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4349/8 -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency accross service
Huaisi Xu has posted comments on this change.
Change subject: IMPALA-1702: Enforce table level consistency accross service
..
Patch Set 7:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/4349/7/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
File fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java:
Line 154: public TDescriptorTable toThrift() {
> Let's add a comment about table id assignment and that this also checks the
Done
Line 156: // Maps from base table to its table id used in backend.
> in the backend
Done
http://gerrit.cloudera.org:8080/#/c/4349/7/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java:
Line 42: import com.google.common.base.Preconditions;
> remove
Done
http://gerrit.cloudera.org:8080/#/c/4349/7/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
Line 1128:* 1. All tables sent to backend have a unique Id, assigned in
> Let's rework/move this validation as follows:
Done
Line 1134: private void validateTableIds(Analyzer analyzer, TExecRequest
result)
> validateTableConsistency()?
Done
http://gerrit.cloudera.org:8080/#/c/4349/7/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java:
Line 93: private final Map tableMap_ =
Maps.newHashMap();
> Integer
Done
--
To view, visit http://gerrit.cloudera.org:8080/4349
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Tim Armstrong
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency accross service
Huaisi Xu has uploaded a new patch set (#7). Change subject: IMPALA-1702: Enforce table level consistency accross service .. IMPALA-1702: Enforce table level consistency accross service Problems: 1. CatalogServiceCatalog::reset() resets table ID, so same tables can have different ID and different tables can have same table ID before/after this call. Backend could overwrites each table in an 1:1 table ID to table descriptor map. 2. 1 can happen because frontend gets table directly from catalog cache, which is updated concurrently with query analyzing. so Analyzer::getTable() could return different references for the same table when analyzing the same query. This commit removes table Id from Table object; instead frontend assigns a unique Id to each table just before it is sent to backend. It also implements a referecedTables_ cache in Analyzer::globalState_ so that calling Analyzer::getTable() on the same table returns the same references for the same query. Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 --- M common/thrift/CatalogObjects.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/TableName.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java D fe/src/main/java/org/apache/impala/catalog/TableId.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/common/Id.java M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/JoinTableId.java M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java 27 files changed, 155 insertions(+), 188 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4349/7 -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency accross service
Huaisi Xu has posted comments on this change.
Change subject: IMPALA-1702: Enforce table level consistency accross service
..
Patch Set 7:
(16 comments)
http://gerrit.cloudera.org:8080/#/c/4349/6/common/thrift/Types.thrift
File common/thrift/Types.thrift:
Line 22: typedef i32 TPlanNodeId
> We can also move this typedef into a more appropriate place like Descriptor
I think it is ok to push it here to stick with others. just make it an int.
http://gerrit.cloudera.org:8080/#/c/4349/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
Line 70: import org.apache.impala.util.DisjointSet;
> will remove
Done
http://gerrit.cloudera.org:8080/#/c/4349/6/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
File fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java:
Line 50: public static final int TABLE_SINK_ID = 0;
> I know it doesn't matter but we typically do "public static final ..."
Done
Line 52: private int nextTableId_ = TABLE_SINK_ID + 1;
> nextTableId_
Done
Line 54: public TupleDescriptor createTupleDescriptor(String debugName) {
> I think we can just remove this and inline at the single caller
Done
Line 158: if (targetTable_ != null) tableIdMap.put(targetTable_,
TABLE_SINK_ID);
> Maps from base table to its table id.
Done
Line 165: Integer tableId = tableIdMap.get(table);
> Lots of nesting here, can we invert this condition, i.e.
Done
Line 166: if (table != null && !(table instanceof View) && tableId ==
null) {
> Remember the tableId here so avoid some hash lookups.
Done
Line 168: tableIdMap.put(table, tableId);
> Use this pattern to avoid multiple lookups:
Done
http://gerrit.cloudera.org:8080/#/c/4349/6/fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
File fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java:
Line 198: public TTupleDescriptor toThrift(Integer tableId) {
> just tableId
Done
http://gerrit.cloudera.org:8080/#/c/4349/6/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
Line 34: import java.util.concurrent.locks.ReentrantReadWriteLock;
> will remove
Done
http://gerrit.cloudera.org:8080/#/c/4349/6/fe/src/main/java/org/apache/impala/catalog/HBaseTable.java
File fe/src/main/java/org/apache/impala/catalog/HBaseTable.java:
Line 665: public TTableDescriptor toThriftDescriptor(int tableId, Set
referencedPartitions) {
> tableId
Done
http://gerrit.cloudera.org:8080/#/c/4349/6/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:
Line 1567: public TTableDescriptor toThriftDescriptor(int tableId, Set
referencedPartitions) {
> tableId
Done
http://gerrit.cloudera.org:8080/#/c/4349/6/fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java
File fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java:
Line 68: public TTableDescriptor toThriftDescriptor(int tableId, Set
referencedPartitions) {
> tableId
Done
http://gerrit.cloudera.org:8080/#/c/4349/6/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
Line 38: import org.apache.impala.catalog.KuduTable;
> not intentional.. will revert next patch!
Done
PS6, Line 1105: String db = insertStmt.getTargetTableName().getDb();
: finalizeParams.setTable_db(db == null ?
queryCtx.session.database : db);
: HdfsTable hdfsTable = (HdfsTable)
insertStmt.getTargetTable();
:
finalizeParams.setHdfs_base_dir(hdfsTable.getHdfsBaseDir());
: finalizeParams.setStaging_dir(
: hdfsTable.getHdfsBaseDir() +
"/_impala_insert_staging");
:
> will update
Done
--
To view, visit http://gerrit.cloudera.org:8080/4349
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Tim Armstrong
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency accross service
Huaisi Xu has posted comments on this change. Change subject: IMPALA-1702: Enforce table level consistency accross service .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/4349/6/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: PS6, Line 1105: /** :* Our single query table consistency guarantee: :* 1. All referenced tables have a unique id :* 2. All references of the same table refer to the same version(indicated by the table :* id). :* Validate 2 by checking same tables have the same table instance. :*/ will update -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency accross service
Huaisi Xu has posted comments on this change. Change subject: IMPALA-1702: Enforce table level consistency accross service .. Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/4349/6/common/thrift/Types.thrift File common/thrift/Types.thrift: Line 22: typedef i64 TTableId now i32 is ok I think. http://gerrit.cloudera.org:8080/#/c/4349/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: Line 70: import org.apache.impala.thrift.TTableName; will remove http://gerrit.cloudera.org:8080/#/c/4349/6/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: Line 34: import java.util.concurrent.atomic.AtomicLong; will remove http://gerrit.cloudera.org:8080/#/c/4349/6/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: Line 38: import org.apache.impala.analysis.*; not intentional.. will revert next patch! -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency accross service
Huaisi Xu has posted comments on this change. Change subject: IMPALA-1702: Enforce table level consistency accross service .. Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/4349/4/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java File fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java: Line 44: // The output table of an insert query. > I still don't understand what this has to do with partition pruning. The pu Done http://gerrit.cloudera.org:8080/#/c/4349/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: Line 34: import java.util.concurrent.atomic.AtomicLong; remove next patch Line 141: private final TableLoadingMgr tableLoadingMgr_; > Sorry I got confused, you can remove this sentence. I incorrectly thought t done http://gerrit.cloudera.org:8080/#/c/4349/4/fe/src/main/java/org/apache/impala/catalog/Table.java File fe/src/main/java/org/apache/impala/catalog/Table.java: Line 64: > The metastoreAccessLock_ is unused. Please remove. Done http://gerrit.cloudera.org:8080/#/c/4349/5/fe/src/main/java/org/apache/impala/catalog/Table.java File fe/src/main/java/org/apache/impala/catalog/Table.java: Line 79: protected final ArrayList colsByPos_ = Lists.newArrayList(); > As discussed, thinking about this a little more I think the right fix is to kind of done. what do you think -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency accross service
Huaisi Xu has uploaded a new patch set (#6). Change subject: IMPALA-1702: Enforce table level consistency accross service .. IMPALA-1702: Enforce table level consistency accross service Problems: 1. CatalogServiceCatalog::reset() resets table ID, so same tables can have different ID and different tables can have same table ID before/after this call. Backend could overwrites each table in an 1:1 table ID to table descriptor map. 2. 1 can happen because frontend gets table directly from catalog cache, which is updated concurrently with query analyzing. so Analyzer::getTable() could return different references for the same table when analyzing the same query. This commit removes table Id, and keeps a referecedTables_ cache in Analyzer::globalState_ so that calling Analyzer::getTable() on the same table returns the same references for the same query. Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 --- M common/thrift/CatalogObjects.thrift M common/thrift/Types.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/TableName.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java D fe/src/main/java/org/apache/impala/catalog/TableId.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/common/Id.java M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/JoinTableId.java M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java 28 files changed, 151 insertions(+), 201 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4349/6 -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency accross service
Huaisi Xu has uploaded a new patch set (#5). Change subject: IMPALA-1702: Enforce table level consistency accross service .. IMPALA-1702: Enforce table level consistency accross service Problems: 1. CatalogServiceCatalog::reset() resets table ID, so same tables can have different ID and different tables can have same table ID before/after this call. Backend could overwrites each table in an 1:1 table ID to table descriptor map. 2. 1 can happen because frontend gets table directly from catalog cache, which is updated concurrently with query analyzing. so Analyzer::getTable() could return different references for the same table when analyzing the same query. This commit solves different table having same table ID issue by changing table ID counter to an ever increasing 64 bits integer. It solves same table with different table issue ID by keeping a referecedTables_ cache in Analyzer::globalState_ so that calling Analyzer::getTable() on the same table returns the same references for the same query. Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 --- M common/thrift/Types.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java D fe/src/main/java/org/apache/impala/catalog/TableId.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/common/Id.java M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/JoinTableId.java M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java 26 files changed, 118 insertions(+), 122 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4349/5 -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency accross service
Huaisi Xu has posted comments on this change.
Change subject: IMPALA-1702: Enforce table level consistency accross service
..
Patch Set 4:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/4349/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
Line 294: // Catalog cache has the latest tables from the statestore, in
order to use the
> The Impalad Catalog has ...
Done
Line 295: // same table version in a single query, we cache all referenced
tables and reuse them
> ... query , we cache all referenced tables here.
Done
Line 2299: if (table == null) {
> remove
Done
http://gerrit.cloudera.org:8080/#/c/4349/4/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java:
Line 187: // SelectStmt (or the BE will be very confused). Assign a
unique table ID to it.
> The previous longer comment was clearer. How about:
Done
http://gerrit.cloudera.org:8080/#/c/4349/4/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
File fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java:
Line 44: // List of tables to exclude from partition pruning, and thus all
partitions are sent to
> Afaik, these tables have nothing to do with partition pruning, so the name
The only place that this actually becomes useful is for partition pruning? so
for "INSERT INTO a PARTITION(b='d') select c from a;
if c is in another partition other than partition 'b', then without this
partition b's metadata wont be sent. it has no other meaningful usage than this
I think. maybe I just change this to a HashSet?
Line 181: referencedPartitions = getReferencedPartitions(tbl);
> will remove this
Done
http://gerrit.cloudera.org:8080/#/c/4349/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
Line 141:* Ever increasing counter for table id {@link Table#id_}. New id
is given when
> TableId doesn't exist anymore.
Done
http://gerrit.cloudera.org:8080/#/c/4349/4/fe/src/main/java/org/apache/impala/catalog/Table.java
File fe/src/main/java/org/apache/impala/catalog/Table.java:
Line 64: private static final Object metastoreAccessLock_ = new Object();
> Please remove while you are here.
remove which? ?removed INVALID_TABLE_ID. not sure if this is what you wanted.
Line 72: public static final long LOCAL_TABLE_ID = -4;
> LOCAL_VIEW_ID is more explicit. Sorry to make you change it again.
Done
http://gerrit.cloudera.org:8080/#/c/4349/4/fe/src/main/java/org/apache/impala/planner/JoinTableId.java
File fe/src/main/java/org/apache/impala/planner/JoinTableId.java:
Line 25: // Construction only allowed via an IdGenerator.
> newline before comment
Done
http://gerrit.cloudera.org:8080/#/c/4349/4/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
Line 1126:* Check that we don't have any duplicate table IDs (see
IMPALA-1702).
> We should declare IMPALA-1702 as fixed and not mention it here. Instead, we
I added something in ImpaladCatalog.java::addTable(). not sure if that is what
you suggested.k
--
To view, visit http://gerrit.cloudera.org:8080/4349
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Tim Armstrong
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency accross service
Huaisi Xu has posted comments on this change. Change subject: IMPALA-1702: Enforce table level consistency accross service .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/4349/4/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java File fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java: Line 181: referencedPartitions = getReferencedPartitions(tbl); will remove this -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency accross service
Huaisi Xu has posted comments on this change.
Change subject: IMPALA-1702: Enforce table level consistency accross service
..
Patch Set 4:
(17 comments)
http://gerrit.cloudera.org:8080/#/c/4349/3//COMMIT_MSG
Commit Message:
PS3, Line 12: all. Backend could
: overwrites each table in an 1:1 table ID to table descriptor
: map.
> might need a little rephrase.
what about this?
PS3, Line 18: analyzing.
> operations?
Done
PS3, Line 22:
> ..issue.. (same in next statement too.)
Done
http://gerrit.cloudera.org:8080/#/c/4349/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
Line 294: // Catalog cache has the latest tables from the statestore, in
order to use the
> "Caches all tables referenced in a single query"
Done
PS3, Line 296: same qu
> I think its better to use the parent Map<> class here (that seems to be con
I saw a lot of HashMap in the code as well?
PS3, Line 2282:
:* Returns the Catalog Table object for the given database
and table name. The same
:* table reference is returned for multiple references in the
same query.
:* Adds the table to this analyzer's "missingTbls_" and
throws an AnalysisException if
:* the table has not yet been loaded in the local catalog
cache.
> Please update this to reflect the new logic.
Done
Line 2290: public Table getTable(String dbName, String tableName)
> I'm not sure if this will deal with case-insensitivity properly. Why not us
Done
Line 2291: throws AnalysisException, TableLoadingException {
> How about this:
Done
http://gerrit.cloudera.org:8080/#/c/4349/3/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
File fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java:
PS3, Line 45: mple, the ou
> Just curious what the rationale behind this clean up. Is it because we use
The reason for this is that previously I tried to add the cache here and wanted
to use the name referencedTable_, which was already used.. So I have to give it
another name, which I failed to think of.. I ended up just replace this with
something else, and I am not sure if I should change it back.. the name is
quite confusing for now because we only uses it for insert table. we definitely
should change its name to be something sounds like more specific for partition
pruning.
http://gerrit.cloudera.org:8080/#/c/4349/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
Line 34: import java.util.concurrent.atomic.AtomicLong;
> Unused import now that we switched to Long?
done.. there are many unused import.. I will leave the rest there?
Line 141:* Ever increasing counter for table id {@link Table#id_}. New id
is given when
> Add comment that explains the use and behavior of this id, i.e. the id is e
Done
http://gerrit.cloudera.org:8080/#/c/4349/3/fe/src/main/java/org/apache/impala/catalog/Table.java
File fe/src/main/java/org/apache/impala/catalog/Table.java:
Line 65: // IDs not covered in all other cases.
> Add comment for these subtly different special ids.
Done
Line 67: // ID used for IncompleteTable when it is uninitialized.
> I think we should add LOCAL_TABLE_ID and TEST_TABLE_ID as well and describe
Done
http://gerrit.cloudera.org:8080/#/c/4349/3/fe/src/main/java/org/apache/impala/common/Id.java
File fe/src/main/java/org/apache/impala/common/Id.java:
Line 42:
> revert
Done
Line 47:
> revert
Done
http://gerrit.cloudera.org:8080/#/c/4349/3/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
Line 1130
> I think we should still maintain something like this to check and document
Done. the new guarantee in Impala frontend is that same query uses the same
table reference for the same table. Different tables with same ID is guaranteed
in catalog.
Line 1134
> There is similar code in a few places in the BE. We should remove that, too
as discussed offline. wont do
--
To view, visit http://gerrit.cloudera.org:8080/4349
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Huaisi Xu
Gerrit-Reviewer: Tim Armstrong
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency accross service
Huaisi Xu has uploaded a new patch set (#4). Change subject: IMPALA-1702: Enforce table level consistency accross service .. IMPALA-1702: Enforce table level consistency accross service Problems: 1. CatalogServiceCatalog::reset() resets table ID, so same tables can have different ID and different tables can have same table ID before/after this call. Backend could overwrites each table in an 1:1 table ID to table descriptor map. 2. 1 can happen because frontend gets table directly from catalog cache, which is updated concurrently with query analyzing. so Analyzer::getTable() could return different references for the same table when analyzing the same query. This commit solves different table having same table ID issue by changing table ID counter to an ever increasing 64 bits integer. It solves same table with different table issue ID by keeping a referecedTables_ cache in Analyzer::globalState_ so that calling Analyzer::getTable() on the same table returns the same references for the same query. Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 --- M common/thrift/Types.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java D fe/src/main/java/org/apache/impala/catalog/TableId.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/common/Id.java M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/JoinTableId.java M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java 25 files changed, 121 insertions(+), 131 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4349/4 -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1702: Enforce table level consistency accross service
Huaisi Xu has uploaded a new patch set (#3). Change subject: IMPALA-1702: Enforce table level consistency accross service .. IMPALA-1702: Enforce table level consistency accross service Problems: 1. CatalogServiceCatalog::reset() resets table ID, so same tables can have different ID and different tables can have same table ID before/after this. If backend sees these, it will overwrites each other in an 1:1 table ID to table descriptor map. 2. 1 can happen because frontend gets table directly from catalog cache, which is updated concurrently with other activities, so Analyzer::getTable() can return different references for the same table when analyzing the same query. This commit solves different table having same table ID by changing table ID counter to an ever increasing 64 bits integer. It solves same table with different table ID by keeping a referecedTables_ cache in Analyzer::globalState_ so that calling Analyzer::getTable() on the same table returns the same references for the same query. Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 --- M common/thrift/Types.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java D fe/src/main/java/org/apache/impala/catalog/TableId.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/View.java M fe/src/main/java/org/apache/impala/common/Id.java M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java 24 files changed, 82 insertions(+), 157 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/4349/3 -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1702: Enforce unique table ID
Huaisi Xu has posted comments on this change. Change subject: IMPALA-1702: Enforce unique table ID .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/4349/2/fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java: Line 188: private final HashMap tblIdMap_ = > I think it may be hard to differentiate between a table dropped + added and why we want to differentiate this? we do not do this now as well right? Line 188: private final HashMap tblIdMap_ = > I think that behavior is actually desired. If a table with the same name wa yeah, but ideally any table change should trigger a new analyzing just like incomplete table would do in the middle of analyze. so we do not have the case when for 100 sub queries, half of them are analyzed based on table version a, the second half on version b. and backend ends up with two version of the same table as well. If we keep them the same table ID for the same table name, then at least backend can do things on a single version of table? I know that may not matter.. anyway, I will do what you guys suggested... thanks -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce unique table ID
Huaisi Xu has posted comments on this change. Change subject: IMPALA-1702: Enforce unique table ID .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4349/2/fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java: Line 188: private final HashMap tblIdMap_ = > this is a good idea! I will try to do this. but for 64bit thing I think it Alex, Bharath, another possibility is that given our asymmetry architecture, one impalad can drop and create a new table with the same name. if this table gets updated in a single catalog update to another impalad, then we might see same table with different table ID as well. -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1702: Enforce unique table ID
Huaisi Xu has posted comments on this change. Change subject: IMPALA-1702: Enforce unique table ID .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4349/2/fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java: Line 188: private final HashMap tblIdMap_ = > +1. Given this happens only with "invalidate metadata", how about we change this is a good idea! I will try to do this. but for 64bit thing I think it is not necessary if we reuse table ID(like Alex suggested). the reason is that the new largest possible IDs is lower bounded by our current implementation. we do not have a problem now so probably that can go to another patch. For this one, I think I will only focus on enforcing unique table ID. Thank you both! -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Huaisi Xu Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
