Skip to content

Reset the operation log before fetching new one#4067

Closed
turboFei wants to merge 4 commits intoapache:masterfrom
turboFei:check_status
Closed

Reset the operation log before fetching new one#4067
turboFei wants to merge 4 commits intoapache:masterfrom
turboFei:check_status

Conversation

@turboFei
Copy link
Copy Markdown
Member

@turboFei turboFei commented Jan 3, 2023

Why are the changes needed?

if (normalizedCliConfig.batchOpts.waitCompletion) {
if (log == null || log.getLogRowSet.size == 0) {
batch = batchRestApi.getBatchById(batchId)
if (BatchUtils.isTerminalState(batch.getState)) {
return (batch, true)
}
}
} else {

We need to reset the operation log before fetching new operation log, otherwise, we might meet corner case.

  • The last operation log is not empty
  • we meet log not found exception
  • we can not terminate the loop because the shouldFinishLog is always false

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 locally before make a pull request

@turboFei turboFei changed the title reset the log before fetch log Reset the log before fetching new operation log Jan 3, 2023
@turboFei turboFei self-assigned this Jan 3, 2023
@turboFei turboFei added this to the v1.6.2 milestone Jan 3, 2023
@turboFei turboFei requested a review from ulysses-you January 3, 2023 12:36
@turboFei turboFei changed the title Reset the log before fetching new operation log Reset the operation log before fetching new one Jan 3, 2023
@ulysses-you
Copy link
Copy Markdown
Contributor

ulysses-you commented Jan 4, 2023

thanks, merging to master/branch-1.6

ulysses-you pushed a commit that referenced this pull request Jan 4, 2023
### _Why are the changes needed?_

https://github.com/apache/kyuubi/blob/9342a29284234e21c73f96024a740c6e93e7cb33/kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala#L134-L141

We need to reset the operation log before fetching new operation log, otherwise, we might meet corner case.

- The last operation log is not empty
- we meet log not found exception
- we can not terminate the loop because the `shouldFinishLog` is always false

### _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

- [x] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #4067 from turboFei/check_status.

Closes #4067

b61e14f [fwang12] style
5787ad9 [fwang12] do not fetch again
6dc479d [fwang12] refactor
37a9190 [fwang12] reset the log before fetch log

Authored-by: fwang12 <fwang12@ebay.com>
Signed-off-by: ulysses-you <ulyssesyou@apache.org>
(cherry picked from commit 2c6f17d)
Signed-off-by: ulysses-you <ulyssesyou@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants