You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/06/14 22:39:13 UTC

[GitHub] [iceberg] krisdas opened a new pull request, #5039: Print db and table name while acquiring hive meta-store lock

krisdas opened a new pull request, #5039:
URL: https://github.com/apache/iceberg/pull/5039

   While acquiring hive-metastore lock on an Iceberg table, after every timeout, below [log line](https://github.com/apache/iceberg/blob/98dc9fe8724375e8b932e7e8afdb925c897b4695/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java#L542) is printed, which doesn't have database and table name information. 
   
   `org.apache.iceberg.hive.HiveTableOperations$WaitingForLockException: Waiting for lock.`
   
   After exhausting all the re-try attempt, finally below [log line](https://github.com/apache/iceberg/blob/98dc9fe8724375e8b932e7e8afdb925c897b4695/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java#L561) prints database and table name.
   
   `Retrying task after failure: Timed out after 180133 ms waiting for lock on database.table`
   
   Here we are adding the database and table name in first log line to speed up investigation related with locking.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] krisdas commented on pull request #5039: Print db and table name while acquiring hive meta-store lock

Posted by GitBox <gi...@apache.org>.
krisdas commented on PR #5039:
URL: https://github.com/apache/iceberg/pull/5039#issuecomment-1155903040

   > Question:
   > 
   > https://github.com/apache/iceberg/blob/a5b2c703fb810a6d0731c18056cf287358d15e6c/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java#L563
   > 
   > 
   > And
   > https://github.com/apache/iceberg/blob/a5b2c703fb810a6d0731c18056cf287358d15e6c/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java#L568
   > 
   > These should be logging the database.tablename if lock is not acquired in a timely manner.
   > 
   > is that exception getting swallowed somewhere or the msg not logged from the exception, which includes these details along with the state of the lock.
   
   These logs gets printed, just that, they are printed at the very end, after exhausting all the retries. Here we are trying to add db.table details after each re-try and get the information up in the log file. Currently with just "Waiting for lock" message, we don't get details about, which db.table, the lock is requested.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] krisdas commented on a diff in pull request #5039: Print db and table name while acquiring hive meta-store lock

Posted by GitBox <gi...@apache.org>.
krisdas commented on code in PR #5039:
URL: https://github.com/apache/iceberg/pull/5039#discussion_r897967378


##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##########
@@ -512,6 +513,7 @@ long acquireLock() throws UnknownHostException, TException, InterruptedException
     LockResponse lockResponse = metaClients.run(client -> client.lock(lockRequest));
     AtomicReference<LockState> state = new AtomicReference<>(lockResponse.getState());
     long lockId = lockResponse.getLockid();
+    final Pair<Long, String> lockDetails = Pair.of(lockId, String.format("%s.%s", database, tableName));

Review Comment:
   In current implementation, only a single lock is acquired, so we can use database/tableName directly inside the task/lambda. Since we are using Tasks.foreach, where multiple tasks can run in future, grouped lock id and table info in a pair (to avoid confusion). thoughts?  



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] krisdas commented on a diff in pull request #5039: Print db and table name while acquiring hive meta-store lock

Posted by GitBox <gi...@apache.org>.
krisdas commented on code in PR #5039:
URL: https://github.com/apache/iceberg/pull/5039#discussion_r897464307


##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##########
@@ -512,6 +513,7 @@ long acquireLock() throws UnknownHostException, TException, InterruptedException
     LockResponse lockResponse = metaClients.run(client -> client.lock(lockRequest));
     AtomicReference<LockState> state = new AtomicReference<>(lockResponse.getState());
     long lockId = lockResponse.getLockid();
+    Pair<Long, String> lockDetails = Pair.of(lockId, String.format("%s.%s", database, tableName));

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] pvary commented on pull request #5039: Hive: Print db and table name while acquiring hive meta-store lock

Posted by GitBox <gi...@apache.org>.
pvary commented on PR #5039:
URL: https://github.com/apache/iceberg/pull/5039#issuecomment-1162629299

   Thanks @krisdas for the PR, and for the team for the review comments!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] pvary commented on a diff in pull request #5039: Print db and table name while acquiring hive meta-store lock

Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #5039:
URL: https://github.com/apache/iceberg/pull/5039#discussion_r897984408


##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##########
@@ -512,6 +513,7 @@ long acquireLock() throws UnknownHostException, TException, InterruptedException
     LockResponse lockResponse = metaClients.run(client -> client.lock(lockRequest));
     AtomicReference<LockState> state = new AtomicReference<>(lockResponse.getState());
     long lockId = lockResponse.getLockid();
+    final Pair<Long, String> lockDetails = Pair.of(lockId, String.format("%s.%s", database, tableName));

Review Comment:
   I think the `Tasks.foreach(lockDetails)` is only used to reuse the retry functionality of `Tasks`. So I think it is perfectly fine to keep the simpler implementation and using database/tableName.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] pvary commented on a diff in pull request #5039: Print db and table name while acquiring hive meta-store lock

Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #5039:
URL: https://github.com/apache/iceberg/pull/5039#discussion_r897660436


##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##########
@@ -512,6 +513,7 @@ long acquireLock() throws UnknownHostException, TException, InterruptedException
     LockResponse lockResponse = metaClients.run(client -> client.lock(lockRequest));
     AtomicReference<LockState> state = new AtomicReference<>(lockResponse.getState());
     long lockId = lockResponse.getLockid();
+    final Pair<Long, String> lockDetails = Pair.of(lockId, String.format("%s.%s", database, tableName));

Review Comment:
   I do not think we need this `Pair` object here. Could we just use database/tableName down below?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] dramaticlly commented on a diff in pull request #5039: Print db and table name while acquiring hive meta-store lock

Posted by GitBox <gi...@apache.org>.
dramaticlly commented on code in PR #5039:
URL: https://github.com/apache/iceberg/pull/5039#discussion_r897412042


##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##########
@@ -512,6 +513,7 @@ long acquireLock() throws UnknownHostException, TException, InterruptedException
     LockResponse lockResponse = metaClients.run(client -> client.lock(lockRequest));
     AtomicReference<LockState> state = new AtomicReference<>(lockResponse.getState());
     long lockId = lockResponse.getLockid();
+    Pair<Long, String> lockDetails = Pair.of(lockId, String.format("%s.%s", database, tableName));

Review Comment:
   nit: can make it final



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] abmo-x commented on pull request #5039: Print db and table name while acquiring hive meta-store lock

Posted by GitBox <gi...@apache.org>.
abmo-x commented on PR #5039:
URL: https://github.com/apache/iceberg/pull/5039#issuecomment-1155830618

   Looking at https://github.com/apache/iceberg/blob/a5b2c703fb810a6d0731c18056cf287358d15e6c/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java#L563, it should be logging the database.tablename if lock is not acquired in a timely manner. 
   
   is that exception getting swallowed somewhere or the msg not logged from those exception which include these details along with the state of the lock.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] abmo-x commented on pull request #5039: Print db and table name while acquiring hive meta-store lock

Posted by GitBox <gi...@apache.org>.
abmo-x commented on PR #5039:
URL: https://github.com/apache/iceberg/pull/5039#issuecomment-1155925120

   > > Question:
   > > https://github.com/apache/iceberg/blob/a5b2c703fb810a6d0731c18056cf287358d15e6c/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java#L563
   > > 
   > > And
   > > https://github.com/apache/iceberg/blob/a5b2c703fb810a6d0731c18056cf287358d15e6c/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java#L568
   > > 
   > > These should be logging the database.tablename if lock is not acquired in a timely manner.
   > > is that exception getting swallowed somewhere or the msg not logged from the exception, which includes these details along with the state of the lock.
   > 
   > These logs gets printed, just that, they are printed at the very end, after exhausting all the retries. Here we are trying to add db.table details after each re-try and get the information up in the log file. Currently with just "Waiting for lock" message, we don't get details about, which db.table, the lock is requested.
   
   Ah! That makes sense, thanks for clarifying. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] pvary merged pull request #5039: Hive: Print db and table name while acquiring hive meta-store lock

Posted by GitBox <gi...@apache.org>.
pvary merged PR #5039:
URL: https://github.com/apache/iceberg/pull/5039


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org