Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22930 )

Change subject: WIP: IMPALA-6406: Add CatalogActionLocker for fine grained DDL 
locking
......................................................................


Patch Set 1:

(34 comments)

gerrit-auto-critic failed. You can reproduce it locally using command:

  python3 bin/jenkins/critique-gerrit-review.py --dryrun

To run it, you might need a virtual env with Python3's venv installed.

http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@5399
PS1, Line 5399:    * TODO: update the comments
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java
File fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java:

http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@37
PS1, Line 37:   private static final Logger LOG = 
LoggerFactory.getLogger(CatalogActionLockerTest.class);
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@62
PS1, Line 62:       TestTask(String name, CatalogActionLocker.OperationType 
opType, String db, String table, long sleepBefore, long sleepAfter) {
line too long (131 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@75
PS1, Line 75:           LOG.info("Thread {}: Starting, op={}, db={}, table={}", 
name, opType, db, table != null ? table : "null");
line too long (116 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@85
PS1, Line 85:             LOG.error("Thread {}: Exception for op={}: {}", name, 
opType, e.getMessage(), e);
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@96
PS1, Line 96:                 LOG.error("Thread {}: Exception releasing lock 
for op={}: {}", name, opType, e.getMessage(), e);
line too long (112 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@101
PS1, Line 101:             LOG.info("Thread {}: No lock acquired for op={}, 
skipping release", name, opType);
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@104
PS1, Line 104:           LOG.info("Thread {}: Counted down latch for op={}, 
remaining={}", name, opType, latch.getCount());
line too long (108 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@110
PS1, Line 110:     executor.submit(new TestTask("T1", 
CatalogActionLocker.OperationType.CREATE_TABLE, "mydb", "tbl1", 0, 2000));
line too long (113 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@113
PS1, Line 113:     executor.submit(new TestTask("T2", 
CatalogActionLocker.OperationType.CREATE_TABLE, "mydb", "tbl1", 500, 1000));
line too long (115 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@116
PS1, Line 116:     executor.submit(new TestTask("T3", 
CatalogActionLocker.OperationType.REFRESH_FUNCTIONS, "mydb", null, 600, 1000));
line too long (118 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@119
PS1, Line 119:     executor.submit(new TestTask("T4", 
CatalogActionLocker.OperationType.CREATE_TABLE_BY_EVENT, "mydb", "tbl1", 700, 
500));
line too long (123 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@122
PS1, Line 122:     executor.submit(new TestTask("T5", 
CatalogActionLocker.OperationType.DROP_DB, "mydb", null, 800, 500));
line too long (107 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@125
PS1, Line 125:     executor.submit(new TestTask("T6", 
CatalogActionLocker.OperationType.CREATE_DB, "mydb2", null, 900, 500));
line too long (110 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@128
PS1, Line 128:     executor.submit(new TestTask("T7", 
CatalogActionLocker.OperationType.DROP_TABLE, "mydb", "tbl2", 1000, 500));
line too long (113 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@131
PS1, Line 131:     executor.submit(new TestTask("T8", 
CatalogActionLocker.OperationType.RENAME_TABLE_BY_EVENT, "mydb", "tbl2", 1100, 
500));
line too long (124 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@134
PS1, Line 134:     executor.submit(new TestTask("T9", 
CatalogActionLocker.OperationType.DROP_TABLE_BY_EVENT, "mydb", "tbl2", 1200, 
500));
line too long (122 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@137
PS1, Line 137:     executor.submit(new TestTask("T10", 
CatalogActionLocker.OperationType.CREATE_DB_BY_EVENT, "mydb3", null, 1300, 
500));
line too long (121 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@141
PS1, Line 141:     LOG.info("Latch await finished, allCompleted={}, remaining 
count={}", allCompleted, latch.getCount());
line too long (106 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@169
PS1, Line 169:         
locker.assertAction(CatalogActionLocker.OperationType.CREATE_TABLE, "mydb", 
"tbl1");
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@187
PS1, Line 187:           
locker.releaseAction(CatalogActionLocker.OperationType.CREATE_TABLE, "mydb", 
"tbl1");
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@206
PS1, Line 206:       
locker.assertAction(CatalogActionLocker.OperationType.RENAME_TABLE_BY_EVENT, 
"mydb", "tbl1", "newdb", "tbl2");
line too long (116 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@207
PS1, Line 207:       LOG.info("Acquired RENAME_TABLE_BY_EVENT (S on mydb, X on 
mydb.tbl1, S on newdb, X on newdb.tbl2)");
line too long (106 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@210
PS1, Line 210:       
locker.assertAction(CatalogActionLocker.OperationType.DROP_TABLE_BY_EVENT, 
"mydb", "tbl1");
line too long (97 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@214
PS1, Line 214:       
locker.releaseAction(CatalogActionLocker.OperationType.DROP_TABLE_BY_EVENT, 
"mydb", "tbl1");
line too long (98 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@218
PS1, Line 218:       
locker.assertAction(CatalogActionLocker.OperationType.CREATE_TABLE_BY_EVENT, 
"newdb", "tbl2");
line too long (100 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@222
PS1, Line 222:       
locker.releaseAction(CatalogActionLocker.OperationType.CREATE_TABLE_BY_EVENT, 
"newdb", "tbl2");
line too long (101 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@226
PS1, Line 226:       
locker.releaseAction(CatalogActionLocker.OperationType.RENAME_TABLE_BY_EVENT, 
"mydb", "tbl1", "newdb", "tbl2");
line too long (117 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@232
PS1, Line 232:       LOG.error("Unexpected exception in 
testRenameTableByEventReentrancy: {}", e.getMessage(), e);
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@246
PS1, Line 246:           
locker.assertAction(CatalogActionLocker.OperationType.CREATE_TABLE_BY_EVENT, 
"newdb", "tbl2");
line too long (104 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@247
PS1, Line 247:           LOG.info("Thread T1: Acquired CREATE_TABLE_BY_EVENT (S 
on newdb, X on newdb.tbl2)");
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@258
PS1, Line 258:           
locker.assertAction(CatalogActionLocker.OperationType.RENAME_TABLE_BY_EVENT, 
"mydb", "tbl1", "newdb", "tbl2");
line too long (120 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@277
PS1, Line 277:         
locker.releaseAction(CatalogActionLocker.OperationType.CREATE_TABLE_BY_EVENT, 
"newdb", "tbl2");
line too long (103 > 90)


http://gerrit.cloudera.org:8080/#/c/22930/1/fe/src/test/java/org/apache/impala/catalog/CatalogActionLockerTest.java@287
PS1, Line 287:       LOG.error("Unexpected exception in 
testRenameTableByEventRollback: {}", e.getMessage(), e);
line too long (97 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2ed48d92333787f75b64b8bd878834612433f6f8
Gerrit-Change-Number: 22930
Gerrit-PatchSet: 1
Gerrit-Owner: Yida Wu <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Comment-Date: Wed, 21 May 2025 05:07:52 +0000
Gerrit-HasComments: Yes

Reply via email to