You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "cxzl25 (via GitHub)" <gi...@apache.org> on 2023/04/10 13:15:14 UTC
[GitHub] [kyuubi] cxzl25 opened a new pull request, #4688: Fix the failure to read the operation log after executing Catalog and database operation
cxzl25 opened a new pull request, #4688:
URL: https://github.com/apache/kyuubi/pull/4688
### _Why are the changes needed?_
Now `GetCurrentCatalog`/`GetCurrentDatabase`/`SetCurrentCatalog`/`SetCurrentDatabase`is executed through the statement, and the jdbc client will try to obtain the operation log corresponding to the statement.
At present, these operations do not generate operation logs, so the engine log will be throw exception(`failed to generate operation log`).
```java
23/04/10 20:25:23 INFO GetCurrentCatalog: Processing anonymous's query[8218e7ed-b4a4-41ad-a1cc-6f82bf3d55bb]: INITIALIZED_STATE -> RUNNING_STATE, statement:
GetCurrentCatalog
23/04/10 20:25:23 INFO GetCurrentCatalog: Processing anonymous's query[8218e7ed-b4a4-41ad-a1cc-6f82bf3d55bb]: RUNNING_STATE -> FINISHED_STATE, time taken: 0.002 seconds
23/04/10 20:25:23 ERROR SparkTBinaryFrontendService: Error fetching results:
org.apache.kyuubi.KyuubiSQLException: OperationHandle [8218e7ed-b4a4-41ad-a1cc-6f82bf3d55bb] failed to generate operation log
at org.apache.kyuubi.KyuubiSQLException$.apply(KyuubiSQLException.scala:69)
at org.apache.kyuubi.operation.OperationManager.$anonfun$getOperationLogRowSet$2(OperationManager.scala:146)
at scala.Option.getOrElse(Option.scala:189)
```
### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [ ] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] cxzl25 commented on pull request #4688: Fix the failure to read the operation log after executing catalog and database operation
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on PR #4688:
URL: https://github.com/apache/kyuubi/pull/4688#issuecomment-1503381400
> Can we update the caller side?
If it is to update the operations of the kyuubi jdbc client, the cost of these operations is a bit high.
> Our meta operations do not have operation logs and they work well
Other metadata operations are obtained through the `KyuubiDatabaseMetaData` method, which are directly called through RPC, so there is no need to call `KyuubiStatement` to obtain.
The execution cost of `OperationLog.createOperationLog` is very small.
It just declares a file, it is not actually created, and it does not need to be deleted.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] cxzl25 commented on a diff in pull request #4688: Fix the failure to read the operation log after executing Catalog and database operation
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #4688:
URL: https://github.com/apache/kyuubi/pull/4688#discussion_r1161712456
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/operation/log/OperationLog.scala:
##########
@@ -195,6 +195,8 @@ class OperationLog(path: Path) {
}
def close(): Unit = synchronized {
+ if (!initialized) return
Review Comment:
When the operation log is not initialized, the close method will throw an exception.
```java
Caused by: java.nio.file.NoSuchFileException:
at org.apache.kyuubi.operation.log.OperationLog.reader$lzycompute(OperationLog.scala:89)
at org.apache.kyuubi.operation.log.OperationLog.reader(OperationLog.scala:89)
at org.apache.kyuubi.operation.log.OperationLog.$anonfun$close$1(OperationLog.scala:201)
at org.apache.kyuubi.operation.log.OperationLog.trySafely(OperationLog.scala:221)
```
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] cxzl25 commented on pull request #4688: Fix the failure to read the operation log after executing catalog and database operation
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on PR #4688:
URL: https://github.com/apache/kyuubi/pull/4688#issuecomment-1502626631
> Why do we create operation logs for these lightweight operations?
Because `OperationManager#getOperationLogRowSet` will check whether the operation log has a value, or allow `operationLog` to be none here instead of throwing an exception.
https://github.com/apache/kyuubi/blob/f0615a9aab0a5b755c16ee0b966a5ea59b98bd10/kyuubi-common/src/main/scala/org/apache/kyuubi/operation/OperationManager.scala#L144-L152
> Or why do we fetch logs from the client side?
Because these operations are executed using `KyuubiStatement`, `isLogBeingGenerated` is true by default and will try to fetch query log, which is the default behavior of client.
https://github.com/apache/kyuubi/blob/f0615a9aab0a5b755c16ee0b966a5ea59b98bd10/kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/KyuubiStatement.java#L649-L660
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] yaooqinn commented on pull request #4688: Fix the failure to read the operation log after executing catalog and database operation
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #4688:
URL: https://github.com/apache/kyuubi/pull/4688#issuecomment-1502615491
Or why do we fetch logs from the client side?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] pan3793 closed pull request #4688: Fix the failure to read the operation log after executing catalog and database operation
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 closed pull request #4688: Fix the failure to read the operation log after executing catalog and database operation
URL: https://github.com/apache/kyuubi/pull/4688
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] yaooqinn commented on pull request #4688: Fix the failure to read the operation log after executing catalog and database operation
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #4688:
URL: https://github.com/apache/kyuubi/pull/4688#issuecomment-1502660623
> Because these operations are executed using KyuubiStatement, isLogBeingGenerated is true by default and will try to fetch query log, which is the default behavior of client.
Can we update the caller side? Our meta operations do not have operation logs and they work well
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] yaooqinn commented on pull request #4688: Fix the failure to read the operation log after executing catalog and database operation
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #4688:
URL: https://github.com/apache/kyuubi/pull/4688#issuecomment-1502611240
Why do we create operation logs for these lightweight operations?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] pan3793 commented on pull request #4688: Fix the failure to read the operation log after executing catalog and database operation
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4688:
URL: https://github.com/apache/kyuubi/pull/4688#issuecomment-1508161200
Thanks, merged to master/1.7
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org