seoyoniee opened a new pull request, #4310: URL: https://github.com/apache/amoro/pull/4310
## 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 #4139
`OptimizerController#releaseOptimizer` deleted the resource record and the
optimizer records through two independent transactions:
```java
optimizerManager.deleteResource(resourceId);
optimizerManager.deleteOptimizer(resource.getGroupName(), resourceId);
```
Both records describe the same optimizer, so deleting them in separate
transaction can leave the resource gone while its optimizer rows survive - a
state the release API can no longer recover from.
If the first call committed and the second one failed, the resource row was
deleted while the matching optimizer rows survived. That state cannot be
recovered through the API:
- getResource(resourceId) returns null on the next release attempt, so
resource.getProperties().putAll(...) throws a NullPointerException.
- The container startup stats (yarn-application-id, the Kubernetes namespace
and name, ...) are persisted only in the resource properties, so once that row
is gone the underlying optimizer process cannot be released either.
## Brief change log
<!--
Clearly describe the changes made in modules, classes, methods, etc.
-->
- `OptimizerManager`: rename deleteOptimizer(String, String) to
deleteOptimizerAndResource(String, String), since the operation now owns
both
records. OptimizerController#releaseOptimizer was its only caller;
DefaultOptimizingService#deleteOptimizer is a separate method and is left
untouched.
- `DefaultOptimizerManager#deleteOptimizerAndResource`: run the resource
deletion and
the optimizer deletion inside a single doAsTransaction(...). The existing
deletion
logic is unchanged, only wrapped. Because NestedSqlSession keeps one SQL
session per
thread, the nested doAs commits become no-ops and the outermost
transaction commits
or rolls back both deletions together.
- `OptimizerController#releaseOptimizer`: replace the two calls with the
single
transactional one.
## How was this patch tested?
Should I add a `TestDefaultOptimizerManager` to this PR?
- [ ] 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 making a pull request
## Documentation
- Does this pull request introduce a new feature? (no)
- If yes, how is the feature documented? (not applicable)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
