Aggarwal-Raghav commented on PR #6737:
URL: https://github.com/apache/hive/pull/6737#issuecomment-5703666172
> > > @Aggarwal-Raghav why change existing qfile? it's hard to follow if
there is any regression in q.out
> >
> >
> > @deniskuzZ , The run1 as I mentioned in initial comment is gone from
jenkins. I have attached local q.out diff with just `*.java` changes. The
insert statements / data is controlling the compaction eligibility. As the
double Counting is fixed the more data is required to ensure the compaction
passes.
>
> @Aggarwal-Raghav, could we lower the thresholds for compaction instead of
changing qfiles?
That was my first try but it didn't worked. If I recall , I tried lot of
things but none worked for me. The Amoro calculations (` fragment_size as
target.size / 8`) was very difficult to trigger compaction.
**ok I found my LLM session response that time when I tested:**
```
In Hive's Iceberg/Amoro integration, MINOR compaction only picks up files
that are strictly smaller than a fragment_size threshold. By default,
fragment_size is calculated as target.size / 8.
Here is why your test started returning refused after your Java bug fix:
1. Originally, the test explicitly set target.size = 2920.
2. That made the fragment_size = 2920 / 8 = 365 bytes.
3. Because your 7 new files were ~450 bytes each, they were all strictly
larger than 365 bytes.
4. Amoro's evaluator looked at the files, decided 0 of them were
"fragments", and correctly returned refused.
(Side note: bug fix, they were double-counted so it accidentally bypassed
this check!)
```
```
Why the .q files had to change:
│
│ 1. The Bug: Prior to this PR, CompactionEvaluator had a bug where it
added each input file to the evaluator twice. So, a test that generated 2 files
weighing 400 bytes was being evaluated as 4 files weighing 800 bytes.
│ 2. The Original Tests: The existing tests were originally written with
minimal data that just barely crossed the compaction thresholds (like
min.input.files or target.
│ size), but only because the bug was artificially doubling their counts
and sizes.
│ 3. The Regression: With the bug fixed, the evaluator now correctly sees
the true file counts/sizes. Because these true values fall short of the
thresholds, the tests
│ natively evaluate to MINOR/MAJOR refused.
│ 4. The Solution: The changes to the .q files (splitting inserts, adding
a few rows, or slightly adjusting target.size) simply cross the thresholds
again using the true
│ file counts. This ensures the tests continue to actually exercise the
"compaction succeeded" execution paths rather than skipping them entirely
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
