[
https://issues.apache.org/jira/browse/IMPALA-9062?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16961539#comment-16961539
]
Quanlong Huang commented on IMPALA-9062:
----------------------------------------
Sorry that this is wrong. For local catalog mode, we still need to acquire the
table lock before {{tbl.getCatalogVersion()}}. Because the table version may be
bumpping into the range we are collecting. E.g. in altering table, we increase
and get the catalog version first, then only assign it to the table after the
altering succeeds. Without waiting to acquire the table lock in
{{addTableToCatalogDeltaHelper()}}, we may miss the updated table.
> Don't need to acquire table locks in gathering catalog topic updates in
> minimal topic mode
> ------------------------------------------------------------------------------------------
>
> Key: IMPALA-9062
> URL: https://issues.apache.org/jira/browse/IMPALA-9062
> Project: IMPALA
> Issue Type: Sub-task
> Reporter: Quanlong Huang
> Priority: Major
>
> If catalog_topic_mode is minimal, for table updates, catalogd only propagates
> the database name, table name and catalog version associated with the table:
>
> [https://github.com/apache/impala/blob/3.3.0/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java#L619]
> {code:java}
> private TCatalogObject getMinimalObjectForV2(TCatalogObject obj) {
> Preconditions.checkState(topicMode_ == TopicMode.MINIMAL ||
> topicMode_ == TopicMode.MIXED);
> TCatalogObject min = new TCatalogObject(obj.type, obj.catalog_version);
> switch (obj.type) {
> case DATABASE:
> min.setDb(new TDatabase(obj.db.db_name));
> break;
> case TABLE:
> case VIEW:
> min.setTable(new TTable(obj.table.db_name, obj.table.tbl_name));
> break;{code}
> We acquire the table lock in case of reading partial results written by other
> concurrent DDLs:
> [https://github.com/apache/impala/blob/3.3.0/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java#L1078]
> {code:java}
> private void addTableToCatalogDeltaHelper(Table tbl, GetCatalogDeltaContext
> ctx)
> throws TException {
> TCatalogObject catalogTbl =
> new TCatalogObject(TCatalogObjectType.TABLE,
> Catalog.INITIAL_CATALOG_VERSION);
> tbl.getLock().lock(); <-- acquire table lock here could be blocked by
> DDLs
> try {
> long tblVersion = tbl.getCatalogVersion();
> if (tblVersion <= ctx.fromVersion) return;
> String tableUniqueName = tbl.getUniqueName();
> TopicUpdateLog.Entry topicUpdateEntry =
> topicUpdateLog_.getOrCreateLogEntry(tableUniqueName);
> if (tblVersion > ctx.toVersion &&
> topicUpdateEntry.getNumSkippedTopicUpdates() <
> MAX_NUM_SKIPPED_TOPIC_UPDATES) {
> LOG.info("Table " + tbl.getFullName() + " is skipping topic update " +
> ctx.toVersion);
> topicUpdateLog_.add(tableUniqueName,
> new TopicUpdateLog.Entry(
> topicUpdateEntry.getNumSkippedTopicUpdates() + 1,
> topicUpdateEntry.getLastSentVersion(),
> topicUpdateEntry.getLastSentCatalogUpdate()));
> return;
> }
> try {
> catalogTbl.setTable(tbl.toThrift());
> } catch (Exception e) {
> LOG.error(String.format("Error calling toThrift() on table %s: %s",
> tbl.getFullName(), e.getMessage()), e);
> return;
> }
> catalogTbl.setCatalog_version(tbl.getCatalogVersion());
> ctx.addCatalogObject(catalogTbl, false);
> } finally {
> tbl.getLock().unlock();
> }
> } {code}
> Acquiring the table lock here could be blocked by slow concurrent DDLs like
> REFRESHs, causing problems like IMPALA-6671. Actually in minimal topic mode
> we just need database name, table name and catalog version for a table. The
> first two won't change during DDLs (rename are treated as drop+create). The
> last one, catalog version, is acceptable to propogate a value older than the
> latest version since it's already newer than or equal to those cached in
> coordinators. Thus, we don't need to acquire the table lock here.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
