You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/07/25 06:39:48 UTC
[GitHub] [incubator-kyuubi] ulysses-you opened a new pull request, #3132: [KYUUBI #3131] Improve server operation state change logging
ulysses-you opened a new pull request, #3132:
URL: https://github.com/apache/incubator-kyuubi/pull/3132
<!--
Thanks for sending a pull request!
Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html
2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->
### _Why are the changes needed?_
<!--
Please clarify why the changes are needed. For instance,
1. If you add a feature, you can talk about the use case of it.
2. If you fix a bug, you can clarify why it is a bug.
-->
closes https://github.com/apache/incubator-kyuubi/issues/3131
### _How was this patch tested?_
before:
```
2022-07-25 14:03:31.391 INFO org.apache.kyuubi.operation.ExecuteStatement: Processing anonymous's query[183ee841-16b5-4444-a53f-3e8966e6af69]: INITIALIZED_STATE -> PENDING_STATE, statement: select 1
2022-07-25 14:03:31.410 INFO org.apache.kyuubi.operation.ExecuteStatement: Processing anonymous's query[183ee841-16b5-4444-a53f-3e8966e6af69]: PENDING_STATE -> RUNNING_STATE, statement: select 1
2022-07-25 14:03:31.433 INFO org.apache.kyuubi.operation.ExecuteStatement: Processing anonymous's query[183ee841-16b5-4444-a53f-3e8966e6af69]: RUNNING_STATE -> FINISHED_STATE, statement: select 1, time taken: 0.42 seconds
2022-07-25 14:03:31.446 INFO org.apache.kyuubi.operation.ExecuteStatement: Processing anonymous's query[183ee841-16b5-4444-a53f-3e8966e6af69]: FINISHED_STATE -> CLOSED_STATE, statement: select 1
```
after:
```
2022-07-25 14:29:14.750 INFO org.apache.kyuubi.operation.ExecuteStatement: Processing anonymous's query[22ae85cb-2ce7-4826-ba49-dde87a378c22]: INITIALIZED_STATE -> PENDING_STATE, statement:
select 1
2022-07-25 14:29:14.829 INFO org.apache.kyuubi.operation.ExecuteStatement: Processing anonymous's query[22ae85cb-2ce7-4826-ba49-dde87a378c22]: PENDING_STATE -> RUNNING_STATE, statement:
select 1
2022-07-25 14:29:15.369 INFO org.apache.kyuubi.operation.ExecuteStatement: Processing anonymous's query[22ae85cb-2ce7-4826-ba49-dde87a378c22]: RUNNING_STATE -> FINISHED_STATE, statement:
select 1
time taken: 0.54 seconds
2022-07-25 14:29:15.486 INFO org.apache.kyuubi.operation.ExecuteStatement: Processing anonymous's query[22ae85cb-2ce7-4826-ba49-dde87a378c22]: FINISHED_STATE -> CLOSED_STATE, statement:
select 1
```
--
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] [incubator-kyuubi] ulysses-you commented on pull request #3132: [KYUUBI #3131] Improve operation state change logging
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on PR #3132:
URL: https://github.com/apache/incubator-kyuubi/pull/3132#issuecomment-1198939166
thank you for review, merging to master
--
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] [incubator-kyuubi] codecov-commenter commented on pull request #3132: [KYUUBI #3131] use `\n` to replace `,` in operation state change logging
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #3132:
URL: https://github.com/apache/incubator-kyuubi/pull/3132#issuecomment-1193724083
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/3132?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3132](https://codecov.io/gh/apache/incubator-kyuubi/pull/3132?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6797f94) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/0b3f6b739d3cd253d97a85e142d1d817ff1f6d8b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0b3f6b7) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #3132 +/- ##
=========================================
Coverage 51.31% 51.32%
Complexity 6 6
=========================================
Files 458 458
Lines 25388 25388
Branches 3536 3536
=========================================
+ Hits 13029 13030 +1
- Misses 11115 11116 +1
+ Partials 1244 1242 -2
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/3132?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...rg/apache/kyuubi/operation/AbstractOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3132/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vQWJzdHJhY3RPcGVyYXRpb24uc2NhbGE=) | `81.70% <100.00%> (ø)` | |
| [...org/apache/kyuubi/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3132/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `80.51% <0.00%> (ø)` | |
| [...rg/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3132/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvY21kL2xvZy9Mb2dCYXRjaENvbW1hbmQuc2NhbGE=) | `82.00% <0.00%> (+2.00%)` | :arrow_up: |
Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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] [incubator-kyuubi] ulysses-you commented on a diff in pull request #3132: [KYUUBI #3131] use `\n` to replace `,` in operation state change logging
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #3132:
URL: https://github.com/apache/incubator-kyuubi/pull/3132#discussion_r928719575
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/operation/AbstractOperation.scala:
##########
@@ -103,16 +103,18 @@ abstract class AbstractOperation(session: Session) extends Operation with Loggin
protected def setState(newState: OperationState): Unit = {
OperationState.validateTransition(state, newState)
- var timeCost = ""
newState match {
- case RUNNING => startTime = System.currentTimeMillis()
+ case RUNNING =>
+ info(s"Processing ${session.user}'s query[$statementId]: " +
+ s"${state.name} -> ${newState.name}, statement:\n$redactedStatement")
+ startTime = System.currentTimeMillis()
case ERROR | FINISHED | CANCELED | TIMEOUT =>
completedTime = System.currentTimeMillis()
- timeCost = s", time taken: ${(completedTime - startTime) / 1000.0} seconds"
+ val timeCost = s"\ntime taken: ${(completedTime - startTime) / 1000.0} seconds"
+ info(s"Processing ${session.user}'s query[$statementId]: " +
+ s"${state.name} -> ${newState.name}$timeCost")
Review Comment:
for terminated state, simplify to log user, statementId and timeCost
--
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] [incubator-kyuubi] yaooqinn commented on pull request #3132: [KYUUBI #3131] use `\n` to replace `,` in operation state change logging
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on PR #3132:
URL: https://github.com/apache/incubator-kyuubi/pull/3132#issuecomment-1193654638
> Why are the changes needed?
--
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] [incubator-kyuubi] ulysses-you closed pull request #3132: [KYUUBI #3131] Improve operation state change logging
Posted by GitBox <gi...@apache.org>.
ulysses-you closed pull request #3132: [KYUUBI #3131] Improve operation state change logging
URL: https://github.com/apache/incubator-kyuubi/pull/3132
--
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] [incubator-kyuubi] ulysses-you commented on pull request #3132: [KYUUBI #3131] use `\n` to replace `,` in operation state change logging
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on PR #3132:
URL: https://github.com/apache/incubator-kyuubi/pull/3132#issuecomment-1193796618
> > Why are the changes needed?
@yaooqinn it's for help lookup the log, the statement will print several lines as the screen size is limited. The pr description can not show the issue that the markdown style will always use one line even if the string is long.
--
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] [incubator-kyuubi] yaooqinn commented on pull request #3132: [KYUUBI #3131] use `\n` to replace `,` in operation state change logging
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on PR #3132:
URL: https://github.com/apache/incubator-kyuubi/pull/3132#issuecomment-1193801703
can we print the statement once only in `case RUNNING` branch?
--
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] [incubator-kyuubi] ulysses-you commented on pull request #3132: [KYUUBI #3131] use `\n` to replace `,` in operation state change logging
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on PR #3132:
URL: https://github.com/apache/incubator-kyuubi/pull/3132#issuecomment-1193811931
I think we can, it's real noisy
--
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] [incubator-kyuubi] yaooqinn commented on a diff in pull request #3132: [KYUUBI #3131] Improve operation state change logging
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #3132:
URL: https://github.com/apache/incubator-kyuubi/pull/3132#discussion_r928795345
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/operation/AbstractOperation.scala:
##########
@@ -103,16 +103,18 @@ abstract class AbstractOperation(session: Session) extends Operation with Loggin
protected def setState(newState: OperationState): Unit = {
OperationState.validateTransition(state, newState)
- var timeCost = ""
newState match {
- case RUNNING => startTime = System.currentTimeMillis()
+ case RUNNING =>
+ info(s"Processing ${session.user}'s query[$statementId]: " +
+ s"${state.name} -> ${newState.name}, statement:\n$redactedStatement")
+ startTime = System.currentTimeMillis()
case ERROR | FINISHED | CANCELED | TIMEOUT =>
completedTime = System.currentTimeMillis()
- timeCost = s", time taken: ${(completedTime - startTime) / 1000.0} seconds"
+ val timeCost = s"\ntime taken: ${(completedTime - startTime) / 1000.0} seconds"
Review Comment:
is \n still necessary?
--
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