Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22653 )

Change subject: IMPALA-13884: Add more details in metadata loading logs
......................................................................


Patch Set 6:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/22653/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/22653/5//COMMIT_MSG@27
PS5, Line 27: AlterType. E.g.
            : execDdl req
> Where is this stacktrace printing?
It's done in CatalogServiceCatalog.tryLock():
https://gerrit.cloudera.org/c/22653/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java#636

LOG.warn("{} lock for table {} was acquired in {} msec",
  useWriteLock ? "Write" : "Read", tbl.getFullName(), duration,
  new Throwable());

If the last parameter is a Throwable and not used in the message, its 
stacktrace will be logged.


http://gerrit.cloudera.org:8080/#/c/22653/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/22653/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2644
PS5, Line 2644:           LOG.info("Not updating table {} (transactional). 
Current catalog version: {}." +
              :                   " Expected catalog version: {}. Acid compare: 
{}. Last synced id: {}"
> *to prefix match
Done. Use "transactional" instead of "Hive-ACID" since it can be an insert-only 
transactional table.


http://gerrit.cloudera.org:8080/#/c/22653/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:

http://gerrit.cloudera.org:8080/#/c/22653/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@811
PS5, Line 811:       Object[] formatArgs = new Object[args.length + 3];
             :       formatAr
> Is it possible to leak sensitive information by printing partition value he
We already log partition values in other places, e.g.
"Loaded file and block metadata for tpcds.store_sales partitions: 
ss_sold_date_sk=2450816, ss_sold_date_sk=2450817, ss_sold_date_sk=2450818, and 
1821 others. Time taken: 69.026ms" at
https://github.com/apache/impala/blob/e73e2d40da115ed3804ffaecc0850c853b0e6330/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java#L774-L775

"Refreshing partition metadata: tpcds.store_sales ss_sold_date_sk=2450816 
(REFRESH TABLE tpcds.store_sales PARTITIONS issued by quanlong)" at
https://github.com/apache/impala/blob/e73e2d40da115ed3804ffaecc0850c853b0e6330/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java#L3477-L3478

Some exceptions like FileNotFoundException will also log the file path which 
consists of the partition name.

If we want to redact the partition names, we will need a broader change, 
including redact them in HMS logs.


http://gerrit.cloudera.org:8080/#/c/22653/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@3360
PS5, Line 3360:
> Can we standardize this pattern into a method returning String?
Good point. A method is better.

For the event type strings, some of them have the "_EVENT" suffix:
https://github.com/apache/impala/blob/e73e2d40da115ed3804ffaecc0850c853b0e6330/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java#L135-L138
Removed them to be consistent.


http://gerrit.cloudera.org:8080/#/c/22653/5/fe/src/main/java/org/apache/impala/util/CatalogOpUtil.java
File fe/src/main/java/org/apache/impala/util/CatalogOpUtil.java:

http://gerrit.cloudera.org:8080/#/c/22653/5/fe/src/main/java/org/apache/impala/util/CatalogOpUtil.java@43
PS5, Line 43: getDb();
> nit: Can contain this into a variable first to shorten code.
Done



--
To view, visit http://gerrit.cloudera.org:8080/22653
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I204d5922e055fd8501b5573e3b913f8874d891d6
Gerrit-Change-Number: 22653
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <huangquanl...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com>
Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Mar 2025 00:31:29 +0000
Gerrit-HasComments: Yes

Reply via email to