Skip to content

[KYUUBI #5210][FOLLOWUP] Close batch operation log even batch is in cancel state #5733

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
turboFei wants to merge 1 commit into from
Closed

[KYUUBI #5210][FOLLOWUP] Close batch operation log even batch is in cancel state #5733

turboFei wants to merge 1 commit into from

Conversation

@turboFei
Copy link
Member

@turboFei turboFei commented Nov 20, 2023 •
edited
Loading

🔍 Description

Followup for #5210

For batch operation, it has similar issue.
#5210 (comment)

Issue References 🔗

This pull request fixes #

Describe Your Solution 🔧

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Types of changes 🔖

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests

Checklists

📝 Author Self Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • This patch was not authored or co-authored using Generative Tooling

📝 Committer Pre-Merge Checklist

  • Pull request title is okay.
  • No license issues.
  • Milestone correctly set?
  • Test coverage is ok
  • Assignees are selected.
  • Minimum number of approvals
  • No changes are requested

Be nice. Be informative.

@turboFei turboFei requested a review from wForget November 20, 2023 09:45
@turboFei turboFei added this to the v1.8.1 milestone Nov 20, 2023
@turboFei turboFei closed this in 24fbd57 Nov 20, 2023
turboFei added a commit that referenced this pull request Nov 20, 2023
@turboFei
[KYUUBI #5210][FOLLOWUP] Close batch operation log even batch is in c…
93727f9 
…ancel state

# 🔍 Description

Followup for #5210

For batch operation, it has similar issue.
#5210 (comment)
## Issue References 🔗

This pull request fixes #

## Describe Your Solution 🔧

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

## Types of changes 🔖

- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️

#### Behavior With This Pull Request 🎉

#### Related Unit Tests

---

# Checklists
## 📝 Author Self Checklist

- [x] My code follows the [style guidelines](https://kyuubi.readthedocs.io/en/master/contributing/code/style.html) of this project
- [ ] I have performed a self-review
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

## 📝 Committer Pre-Merge Checklist

- [x] Pull request title is okay.
- [x] No license issues.
- [ ] Milestone correctly set?
- [ ] Test coverage is ok
- [ ] Assignees are selected.
- [ ] Minimum number of approvals
- [ ] No changes are requested

**Be nice. Be informative.**

Closes #5733 from turboFei/batch_log_close.

Closes #5210

e2b72b4 [fwang12] Close batch log even batch is in cancel state

Authored-by: fwang12 <fwang12@ebay.com>
Signed-off-by: fwang12 <fwang12@ebay.com>
(cherry picked from commit 24fbd57)
Signed-off-by: fwang12 <fwang12@ebay.com>
@turboFei
Copy link
Member Author

turboFei commented Nov 20, 2023

thanks, merged to branch-1.8

@turboFei turboFei deleted the batch_log_close branch November 20, 2023 10:04
@bowenliang123
Copy link
Contributor

bowenliang123 commented Dec 14, 2023

image
It's doubted that the steps for closing OperationLog will be skipped when triggering the fast-fail case.

@bowenliang123 bowenliang123 mentioned this pull request Dec 14, 2023
Closed
18 tasks
bowenliang123 added a commit that referenced this pull request Dec 14, 2023
@bowenliang123
[KYUUBI #5853] Ensure closing the operation log for batch submission …
a0fdead 
…in fast-failed case

# 🔍 Description
## Issue References 🔗

This pull request fixes comment in PR [#5733](#5733 (comment))

## Describe Your Solution 🔧

To ensure the operation log closed even in fast-failed case.

## Types of changes 🔖

- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️

#### Behavior With This Pull Request 🎉

#### Related Unit Tests

---

# Checklists
## 📝 Author Self Checklist

- [x] My code follows the [style guidelines](https://kyuubi.readthedocs.io/en/master/contributing/code/style.html) of this project
- [x] I have performed a self-review
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

## 📝 Committer Pre-Merge Checklist

- [ ] Pull request title is okay.
- [ ] No license issues.
- [ ] Milestone correctly set?
- [ ] Test coverage is ok
- [ ] Assignees are selected.
- [ ] Minimum number of approvals
- [ ] No changes are requested

**Be nice. Be informative.**

Closes #5853 from bowenliang123/close-oplog.

Closes #5853

34192fb [Bowen Liang] import
4fe0046 [Bowen Liang] withClosingOperationLog

Authored-by: Bowen Liang <liangbowen@gf.com.cn>
Signed-off-by: liangbowen <liangbowen@gf.com.cn>
bowenliang123 added a commit that referenced this pull request Dec 14, 2023
@bowenliang123
[KYUUBI #5853] Ensure closing the operation log for batch submission …
0d73298 
…in fast-failed case

# 🔍 Description
## Issue References 🔗

This pull request fixes comment in PR [#5733](#5733 (comment))

## Describe Your Solution 🔧

To ensure the operation log closed even in fast-failed case.

## Types of changes 🔖

- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️

#### Behavior With This Pull Request 🎉

#### Related Unit Tests

---

# Checklists
## 📝 Author Self Checklist

- [x] My code follows the [style guidelines](https://kyuubi.readthedocs.io/en/master/contributing/code/style.html) of this project
- [x] I have performed a self-review
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

## 📝 Committer Pre-Merge Checklist

- [ ] Pull request title is okay.
- [ ] No license issues.
- [ ] Milestone correctly set?
- [ ] Test coverage is ok
- [ ] Assignees are selected.
- [ ] Minimum number of approvals
- [ ] No changes are requested

**Be nice. Be informative.**

Closes #5853 from bowenliang123/close-oplog.

Closes #5853

34192fb [Bowen Liang] import
4fe0046 [Bowen Liang] withClosingOperationLog

Authored-by: Bowen Liang <liangbowen@gf.com.cn>
Signed-off-by: liangbowen <liangbowen@gf.com.cn>
(cherry picked from commit a0fdead)
Signed-off-by: liangbowen <liangbowen@gf.com.cn>
zhaohehuhu pushed a commit to zhaohehuhu/incubator-kyuubi that referenced this pull request Mar 21, 2024
@bowenliang123 @zhaohehuhu
[KYUUBI apache#5853] Ensure closing the operation log for batch submi…
176dc8f 
…ssion in fast-failed case

# 🔍 Description
## Issue References 🔗

This pull request fixes comment in PR [apache#5733](apache#5733 (comment))

## Describe Your Solution 🔧

To ensure the operation log closed even in fast-failed case.

## Types of changes 🔖

- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️

#### Behavior With This Pull Request 🎉

#### Related Unit Tests

---

# Checklists
## 📝 Author Self Checklist

- [x] My code follows the [style guidelines](https://kyuubi.readthedocs.io/en/master/contributing/code/style.html) of this project
- [x] I have performed a self-review
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

## 📝 Committer Pre-Merge Checklist

- [ ] Pull request title is okay.
- [ ] No license issues.
- [ ] Milestone correctly set?
- [ ] Test coverage is ok
- [ ] Assignees are selected.
- [ ] Minimum number of approvals
- [ ] No changes are requested

**Be nice. Be informative.**

Closes apache#5853 from bowenliang123/close-oplog.

Closes apache#5853

34192fb [Bowen Liang] import
4fe0046 [Bowen Liang] withClosingOperationLog

Authored-by: Bowen Liang <liangbowen@gf.com.cn>
Signed-off-by: liangbowen <liangbowen@gf.com.cn>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wForget wForget wForget approved these changes

Assignees

No one assigned

Labels

module:server

Projects

None yet

Milestone

v1.8.1

Development

Successfully merging this pull request may close these issues.

3 participants

@turboFei @bowenliang123 @wForget