You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@amoro.apache.org by "lintingbin (via GitHub)" <gi...@apache.org> on 2026/01/21 11:48:01 UTC
[I] [Bug]: The `reachMinorInterval` function may never evaluate to true. [amoro]
lintingbin opened a new issue, #4055:
URL: https://github.com/apache/amoro/issues/4055
### What happened?
```
public boolean isMinorNecessary() {
int smallFileCount = fragmentFileCount + equalityDeleteFileCount;
return smallFileCount >= config.getMinorLeastFileCount()
|| (smallFileCount > 1 && reachMinorInterval())
|| combinePosSegmentFileCount > 0;
}
protected boolean reachMinorInterval() {
return config.getMinorLeastInterval() >= 0
&& planTime - lastMinorOptimizingTime > config.getMinorLeastInterval();
}
```
If a table has some partitions with many small files and others with only two or three small files, the condition `(smallFileCount > 1 && reachMinorInterval())` for those partitions with just two or three small files will never evaluate to true. Consequently, these partitions will never be included in minor optimizations. Essentially, `reachMinorInterval` should be evaluated at the partition level rather than the table level.
### Affects Versions
0.8.1
### What table formats are you seeing the problem on?
Iceberg
### What engines are you seeing the problem on?
Spark
### How to reproduce
_No response_
### Relevant log output
```shell
```
### Anything else
```
protected boolean reachMinorInterval() {
if (config.getMinorLeastInterval() < 0) {
return false;
}
long interval = planTime - lastMinorOptimizingTime;
if (interval > config.getMinorLeastInterval()) {
return true;
}
return isDifferentDay(lastMinorOptimizingTime, planTime);
}
```
Perhaps the reachMinorInterval can be modified to follow the aforementioned logic, ensuring that it evaluates to true at least once per day. This way, partitions with only two or three small files will also have a chance to be optimized.
### Are you willing to submit a PR?
- [x] Yes I am willing to submit a PR!
### Code of Conduct
- [x] I agree to follow this project's Code of Conduct
--
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.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [Bug]: The `reachMinorInterval` function may never evaluate to true. [amoro]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #4055:
URL: https://github.com/apache/amoro/issues/4055#issuecomment-5447672350
This issue has been closed because it has not received any activity in the last 14 days since being marked as 'stale'
--
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: [I] [Bug]: The `reachMinorInterval` function may never evaluate to true. [amoro]
Posted by "vaquarkhan (via GitHub)" <gi...@apache.org>.
vaquarkhan commented on issue #4055:
URL: https://github.com/apache/amoro/issues/4055#issuecomment-3796127462
**Root Cause**
The system currently experiences a "noisy neighbor" effect during minor compaction. In CommonPartitionEvaluator.isMinorNecessary(), the reachMinorInterval check depends on a table-level timestamp (lastMinorOptimizingTime).
Since DefaultTableRuntime.completeProcess() updates this global timestamp whenever any partition completes an optimization, high-traffic partitions effectively "starve" quiet ones. The global clock is reset so frequently that the planTime - lastMinorOptimizingTime delta never reaches the required interval for partitions containing only a few small files.
**Implementation Plan**
We will shift from global tracking to per-partition tracking to ensure scheduling fairness.
**State Updates:**
We should add Map<String, Long> lastMinorOptimizingTimeByPartition to TableRuntimeOptimizingState.
On MINOR run completion, update the map for each involved partition path using the planTime.
Retain the table-level lastMinorOptimizingTime as a fallback/default.
**Planner Logic:**
During the planning phase (where partition paths for getFromSequence / getToSequence are resolved), the planner will perform the map lookup.
The planner will resolve the specific lastMinorOptimizingTime for that partition (defaulting to the table-level value if missing).
This resolved value is then passed directly into the CommonPartitionEvaluator constructor.
**Backward Compatibility:**
Missing map entries will automatically fall back to the table-level timestamp, ensuring no regressions for existing data.
--
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: [I] [Bug]: The `reachMinorInterval` function may never evaluate to true. [amoro]
Posted by "lintingbin (via GitHub)" <gi...@apache.org>.
lintingbin commented on issue #4055:
URL: https://github.com/apache/amoro/issues/4055#issuecomment-3797717717
> Thanks [@vaquarkhan](https://github.com/vaquarkhan) for the detailed analysis! You've correctly identified the "noisy neighbor" problem where active partitions prevent quiet ones from being optimized.
>
> However, I think the per-partition tracking approach might be **overengineered** for this specific issue. Here are my concerns:
>
> ## Issues with Per-Partition Tracking:
> 1. **State management overhead**: For tables with many historical partitions (e.g., daily partitions over years), we'd need to maintain a large map. Most of these entries would be used only once or very rarely, which is wasteful.
> 2. **Memory/storage overhead**: The `Map<String, Long>` would grow indefinitely unless we implement cleanup logic, adding more complexity.
>
> ## Alternative Lightweight Solution:
> I propose a **table-level approach** with a simple tweak to the original issue's suggestion:
>
> protected boolean reachMinorInterval() {
> if (config.getMinorLeastInterval() < 0) {
> return false;
> }
>
> long interval = planTime - lastMinorOptimizingTime;
>
> if (interval > config.getMinorLeastInterval()) {
> return true;
> }
>
> // Ensure minor optimization runs at least once per day
> return isDifferentDay(lastMinorOptimizingTime, planTime);
> }
> **Key advantages:**
>
> * ✅ Simple: No new state storage required
> * ✅ Effective: Ensures quiet partitions get optimized at least once daily
> * ✅ Backward compatible: No schema changes needed
> * ✅ Low overhead: Minimal logic change
>
> **How it solves the problem:** Even if active partitions constantly reset `lastMinorOptimizingTime`, the `isDifferentDay()` check ensures that **at least once per day**, partitions with just 2-3 small files will have a chance to be optimized.
>
> For most use cases, daily optimization of quiet partitions should be sufficient. If needed, the interval can still be configured via `minorLeastInterval` for more frequent optimizations.
>
> What do you think? [@vaquarkhan](https://github.com/vaquarkhan) would this simpler approach work for your use case?
@zhoujinsong What do you think?
--
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: [I] [Bug]: The `reachMinorInterval` function may never evaluate to true. [amoro]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #4055:
URL: https://github.com/apache/amoro/issues/4055#issuecomment-5287848935
This issue has been automatically marked as stale because it has been open for 180 days with no activity. It will be closed in next 14 days if no further activity occurs. To permanently prevent this issue from being considered stale, add the label 'not-stale', but commenting on the issue is preferred when possible.
--
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: [I] [Bug]: The `reachMinorInterval` function may never evaluate to true. [amoro]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed issue #4055: [Bug]: The `reachMinorInterval` function may never evaluate to true.
URL: https://github.com/apache/amoro/issues/4055
--
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: [I] [Bug]: The `reachMinorInterval` function may never evaluate to true. [amoro]
Posted by "lintingbin (via GitHub)" <gi...@apache.org>.
lintingbin commented on issue #4055:
URL: https://github.com/apache/amoro/issues/4055#issuecomment-3797707009
Thanks @vaquarkhan for the detailed analysis! You've correctly identified the "noisy neighbor" problem where active partitions prevent quiet ones from being optimized.
However, I think the per-partition tracking approach might be **overengineered** for this specific issue. Here are my concerns:
## Issues with Per-Partition Tracking:
1. **State management overhead**: For tables with many historical partitions (e.g., daily partitions over years), we'd need to maintain a large map. Most of these entries would be used only once or very rarely, which is wasteful.
2. **Memory/storage overhead**: The `Map<String, Long>` would grow indefinitely unless we implement cleanup logic, adding more complexity.
## Alternative Lightweight Solution:
I propose a **table-level approach** with a simple tweak to the original issue's suggestion:
```java
protected boolean reachMinorInterval() {
if (config.getMinorLeastInterval() < 0) {
return false;
}
long interval = planTime - lastMinorOptimizingTime;
if (interval > config.getMinorLeastInterval()) {
return true;
}
// Ensure minor optimization runs at least once per day
return isDifferentDay(lastMinorOptimizingTime, planTime);
}
```
**Key advantages:**
- ✅ Simple: No new state storage required
- ✅ Effective: Ensures quiet partitions get optimized at least once daily
- ✅ Backward compatible: No schema changes needed
- ✅ Low overhead: Minimal logic change
**How it solves the problem:**
Even if active partitions constantly reset `lastMinorOptimizingTime`, the `isDifferentDay()` check ensures that **at least once per day**, partitions with just 2-3 small files will have a chance to be optimized.
For most use cases, daily optimization of quiet partitions should be sufficient. If needed, the interval can still be configured via `minorLeastInterval` for more frequent optimizations.
What do you think? @lintingbin would this simpler approach work for your use case?
--
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: [I] [Bug]: The `reachMinorInterval` function may never evaluate to true. [amoro]
Posted by "lintingbin (via GitHub)" <gi...@apache.org>.
lintingbin commented on issue #4055:
URL: https://github.com/apache/amoro/issues/4055#issuecomment-3797713918
Sorry for the confusion in my previous comment - I shouldn't have asked myself! 😅
@vaquarkhan I'd like to hear your thoughts on this simpler table-level approach vs. the per-partition tracking you proposed. Do you think the daily optimization guarantee would be sufficient for most use cases?
--
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: [I] [Bug]: The `reachMinorInterval` function may never evaluate to true. [amoro]
Posted by "vaquarkhan (via GitHub)" <gi...@apache.org>.
vaquarkhan commented on issue #4055:
URL: https://github.com/apache/amoro/issues/4055#issuecomment-3900878476
Agreed please go ahead
--
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