You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2019/04/25 08:53:14 UTC

[Impala-ASF-CR] IMPALA-8454 (part 1): Refactor file descriptor loading code

Hello Bharath Vissapragada, Yongzhi Chen, Vihang Karajgaonkar, Sudhanshu Arora, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12950

to look at the new patch set (#5).

Change subject: IMPALA-8454 (part 1): Refactor file descriptor loading code
......................................................................

IMPALA-8454 (part 1): Refactor file descriptor loading code

This refactors various file-descriptor loading code out of HdfsTable
into new standalone classes. In order to support ACID tables, we'll need
to make various changes to these bits of code, and having them extracted
and cleaned up will make that easier.

This consolidates all of the places in which we list partition
directories into one method which does the appropriate thing regardless
of situation.

This has a small behavior change related to IMPALA-8406: previously, we
had a bug where, while refreshing a table, if one or more partitions
failed to refresh, the other partitions might still get refreshed
despite an error being returned. Those other partitions wouldn't be
available immediately until some other operation caused the table's
catalog version number to increase. This was buggy behavior.

Rather than tackle that problem in this "refactor" patch, this patch
just slightly improves the behavior: we'll either atomically update or
not update all partitions, but we might still add new partitions noticed
by the REFRESH, and might still update other HMS metadata.

This patch may end up slightly improving various other code paths that
refresh file descriptor lists. We used to have slightly different ways
of doing this in three different places, with different sets of
optimizations. Now we do it all in one place, and we pull all the known
tricks.

Change-Id: I59edf493b9ba38be5f556b4795a7684d9c9e3a07
---
A fe/src/main/java/org/apache/impala/catalog/FileMetadataLoader.java
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
A fe/src/main/java/org/apache/impala/catalog/ParallelFileMetadataLoader.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
M fe/src/test/java/org/apache/impala/catalog/HdfsPartitionTest.java
M fe/src/test/java/org/apache/impala/planner/TestCaseLoaderTest.java
10 files changed, 478 insertions(+), 466 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/12950/5
-- 
To view, visit http://gerrit.cloudera.org:8080/12950
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I59edf493b9ba38be5f556b4795a7684d9c9e3a07
Gerrit-Change-Number: 12950
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>