apache / apache/datafusion-comet

Establish whether the DataFusion 55 TPC-H off-heap bump is real, and changelog it if so

Closed
#5,704 1 comment 0 reactions 0 assignees View on GitHub
area:aggregation enhancement priority:medium requires-triage
Dominant language
Scala
Stars
1.3k
Forks
373
Avg merge
2d 4h
Merged PRs (30d)
198

Description

### Describe the bug

The DataFusion 55 branch raises the off-heap size in `CometTPCHQuerySuite` from 2g to 4g:

```scala
conf.set(MEMORY_OFFHEAP_SIZE.key, "4g")
```

It is not established whether DataFusion 55 genuinely needs more memory for these queries, or whether this is a consequence of the `peak_mem_used` accounting change in apache/datafusion#22710 that also disabled two `CometAggregateSuite` metric tests.

### Expected behavior

If DataFusion 55 really does need more off-heap for TPC-H, that is a user-visible change and belongs in the changelog: anyone who sized their off-heap against TPC-H would be under-provisioned after upgrading. If it is only an accounting artifact, the bump should be reverted along with the metric fix.

### Additional context

Found while reviewing #5262. Related to the ignored aggregate metric tests.

Contributor guide

Open the contributing guide

Research direction

Start with the DataFusion 55 change in CometTPCHQuerySuite where MEMORY_OFFHEAP_SIZE is raised from 2g to 4g, then review the disabled metric tests in CometAggregateSuite and the peak_mem_used accounting change in apache/datafusion#22710. Compare whether TPC-H genuinely requires the larger limit or whether the bump is an accounting artifact; changelog the user-visible increase, or revert it with the metric fix if not.

Written by the indexing model from the issue text.

Assessment

Tech stack
scala, spark
Domain
performance, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.