You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@amoro.apache.org by "7hong (via GitHub)" <gi...@apache.org> on 2024/12/16 02:18:19 UTC
[PR] [AMORO-3365]The table is always in committing state [amoro]
7hong opened a new pull request, #3366:
URL: https://github.com/apache/amoro/pull/3366
<!--
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://amoro.apache.org/how-to-contribute/
2. If the PR is related to an issue in https://github.com/apache/amoro/issues, add '[AMORO-XXXX]' in your PR title, e.g., '[AMORO-XXXX] Your PR title ...'.
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][AMORO-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 its use case.
2. If you fix a bug, you can clarify why it is a bug.
3. Use Fix/Resolve/Close #{ISSUE_NUMBER} to link this PR to its related issue
-->
Close #3365.
--
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: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3365]The table is always in committing state [amoro]
Posted by "zhoujinsong (via GitHub)" <gi...@apache.org>.
zhoujinsong merged PR #3366:
URL: https://github.com/apache/amoro/pull/3366
--
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: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3365]The table is always in committing state [amoro]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3366:
URL: https://github.com/apache/amoro/pull/3366#issuecomment-2601082314
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@amoro.apache.org list. Thank you for your contributions.
--
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: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3365]The table is always in committing state [amoro]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3366:
URL: https://github.com/apache/amoro/pull/3366#issuecomment-2556173540
## [Codecov](https://app.codecov.io/gh/apache/amoro/pull/3366?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `37.50000%` with `5 lines` in your changes missing coverage. Please review.
> Project coverage is 27.48%. Comparing base [(`243d289`)](https://app.codecov.io/gh/apache/amoro/commit/243d2894c6bbba59b74be97a1ea93f935f0bd9ad?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`3fcc6ac`)](https://app.codecov.io/gh/apache/amoro/commit/3fcc6ac6d32480916d3468ac1e7985a70ce6c50c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 15 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/amoro/pull/3366?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...pache/amoro/server/optimizing/OptimizingQueue.java](https://app.codecov.io/gh/apache/amoro/pull/3366?src=pr&el=tree&filepath=amoro-ams%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Famoro%2Fserver%2Foptimizing%2FOptimizingQueue.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-YW1vcm8tYW1zL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9hbW9yby9zZXJ2ZXIvb3B0aW1pemluZy9PcHRpbWl6aW5nUXVldWUuamF2YQ==) | 37.50% | [4 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/amoro/pull/3366?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #3366 +/- ##
============================================
+ Coverage 21.59% 27.48% +5.89%
- Complexity 2309 3511 +1202
============================================
Files 426 593 +167
Lines 39719 48310 +8591
Branches 5624 6234 +610
============================================
+ Hits 8577 13280 +4703
- Misses 30414 34096 +3682
- Partials 728 934 +206
```
| [Flag](https://app.codecov.io/gh/apache/amoro/pull/3366/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [core](https://app.codecov.io/gh/apache/amoro/pull/3366/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `27.48% <37.50%> (?)` | |
| [trino](https://app.codecov.io/gh/apache/amoro/pull/3366/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/amoro/pull/3366?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3365]The table is always in committing state [amoro]
Posted by "zhoujinsong (via GitHub)" <gi...@apache.org>.
zhoujinsong commented on PR #3366:
URL: https://github.com/apache/amoro/pull/3366#issuecomment-2556225521
Thanks for the contribution!
Here are my thoughts:
- The commits on the Iceberg table and the commits in the database are two separate processes. This issue is somewhat similar to a distributed transaction problem.
- If the commit on the Iceberg table is successful but the database commit fails, we should attempt to retry until the database write is successful.
- We should avoid making duplicate commits on the Iceberg table, as some repeated commits may lead to data duplication issues.
So the question arises:
- when re-entering the commit process, how do we determine whether the commit on Iceberg has already been completed?
I am afraid the current implementation will fail when the AMS restart.
--
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: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3365]The table is always in committing state [amoro]
Posted by "zhoujinsong (via GitHub)" <gi...@apache.org>.
zhoujinsong commented on PR #3366:
URL: https://github.com/apache/amoro/pull/3366#issuecomment-2603975386
Hi，
A possible resolution is to attempt to close processes that are in a committing state during AMS restart process, in order to avoid duplicate commit on the table. This modification requires minimal changes to the code, although it may waste already completed files. However, the triggering probability is low, and I think it is a viable solution.
--
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: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3365]The table is always in committing state [amoro]
Posted by "zhoujinsong (via GitHub)" <gi...@apache.org>.
zhoujinsong commented on PR #3366:
URL: https://github.com/apache/amoro/pull/3366#issuecomment-2606576081
Hi, some unit test cases should be adjusted due to this change, like `TestDefaultOptimizingService#testReloadCompletedTask` would be changed to :
```java
@Test
public void testReloadCompletedTask() {
// THREAD_ID.poll task
OptimizingTask task = optimizingService().pollTask(token, THREAD_ID);
Assertions.assertNotNull(task);
optimizingService().ackTask(token, THREAD_ID, task.getTaskId());
optimizingService().completeTask(token, buildOptimizingTaskResult(task.getTaskId()));
reload();
// Committing process will be closed when reloading
Assertions.assertNull(
tableService()
.getRuntime(serverTableIdentifier().getId())
.getOptimizingProcess());
Assertions.assertEquals(
OptimizingStatus.IDLE,
tableService().getRuntime(serverTableIdentifier().getId()).getOptimizingStatus());
Assertions.assertNull(optimizingService().pollTask(token, THREAD_ID));
}
```
--
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: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3365]The table is always in committing state [amoro]
Posted by "7hong (via GitHub)" <gi...@apache.org>.
7hong commented on PR #3366:
URL: https://github.com/apache/amoro/pull/3366#issuecomment-2606615110
OK, thank you very much, I will modify it later.
--
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: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org