Add physical data scan tracking to resource groups #25113

kekwan · 2025-02-21T23:22:57Z

Description

This PR adds support to track physical data scan usage in resource groups. This is an optional feature in resource groups and behaves similar to cpuLimit. This is a helpful feature for admins who need to 'throttle' their users from pulling too much data from file system. Quota based tracking will penalize users accordingly.

Additional context and related issues

#25003

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

## General
* Add physical data scan tracking to resource groups({issue}`25003`)
## SPI
*  Methods added to io.trino.spi.resourcegroups for tracking data scan usage({issue}`25003`)

kekwan · 2025-03-05T18:31:07Z

Hi @hashhar. Just a gentle reminder on this PR in case you forgot. Thank you!

github-actions · 2025-05-19T17:03:22Z

This pull request has gone a while without any activity. Ask for help on #core-dev on Trino slack.

github-actions · 2025-06-09T17:03:33Z

Closing this pull request, as it has been stale for six weeks. Feel free to re-open at any time.

kekwan · 2025-06-09T17:05:52Z

@piotrrzysko @hashhar @lukasz-stec @wendigo Could I get a review here?

core/trino-main/src/main/java/io/trino/execution/resourcegroups/InternalResourceGroup.java

core/trino-spi/src/main/java/io/trino/spi/resourcegroups/ResourceGroup.java

core/trino-main/src/main/java/io/trino/execution/resourcegroups/InternalResourceGroup.java

...gers/src/main/resources/db/migration/mysql/V8__add_physical_data_scan_to_resource_groups.sql

kekwan · 2025-07-24T21:00:26Z

...rc/test/java/io/trino/plugin/resourcegroups/db/TestDbResourceGroupsMysqlFlywayMigration.java

    }
-
-    @Test
-    public void testMigrationWithOldResourceGroupsSchema()


I removed this test as it is not compatible with the new constraint. I feel this test is unnecessary.

I'm not sure why only this MySQL test supports old data during the migration. Postgres and Oracle tests do not have this test.

kekwan · 2025-07-24T21:08:04Z

@lukasz-stec I have made the changes to support period-based tracking by adding a new property physicalDataScanQuotaPeriod. Please take a look, thanks.

lukasz-stec

Looks good % minor comments

core/trino-main/src/main/java/io/trino/execution/resourcegroups/InternalResourceGroup.java

lukasz-stec · 2025-07-25T14:06:20Z

core/trino-main/src/main/java/io/trino/execution/resourcegroups/ResourceUsage.java

 {
    private final long cpuUsageMillis;
    private final long memoryUsageBytes;
+    private final long physicalDataScanUsageBytes;


To be consistent with the source of this metric (getPhysicalInputDataSize), rename it please to something the has physicalInput in the name e.g. physicalInputDataUsageBytes or physicalInputUsageBytes. Here and everywhere else.

Sounds good. All the internal references of the data usage now use physicalInputDataUsageBytes.

For the user exposed limits and quota period, those still have physical data scan in the naming which I think is fine. It is consistent with settings like query.max-scan-physical-bytes

core/trino-main/src/main/java/io/trino/execution/resourcegroups/InternalResourceGroup.java

xkrogen

Thanks for working on this feature, I think it's a great enhancement!

...trino-main/src/main/java/io/trino/execution/resourcegroups/InternalResourceGroupManager.java

docs/src/main/sphinx/admin/resource-groups.md

...source-group-managers/src/main/java/io/trino/plugin/resourcegroups/db/ResourceGroupsDao.java

core/trino-main/src/main/java/io/trino/server/ResourceGroupInfo.java

core/trino-main/src/main/java/io/trino/execution/resourcegroups/InternalResourceGroup.java

kekwan · 2025-07-25T21:02:01Z

@xkrogen @lukasz-stec thanks for the review, i've addressed all the comments above

lukasz-stec

@kekwan Looks good generally. Please squash the commits as all 3 are tightly related.
Great work!
We also need a maintainer review. @hashhar or @wendigo, Can one of you give it a go?

...rs/src/main/java/io/trino/plugin/resourcegroups/db/ResourceGroupGlobalPropertiesReducer.java

Add physical data scan tracking to resource groups. This is an optional feature in resource groups that tracks data scan via quota based tracking.

wendigo · 2025-08-06T06:21:30Z

core/trino-main/src/main/java/io/trino/execution/resourcegroups/ResourceUsage.java

    {
        this.cpuUsageMillis = cpuUsageMillis;
        this.memoryUsageBytes = memoryUsageBytes;
+        this.physicalInputDataUsageBytes = physicalInputDataUsageBytes;


nit: should we validate that these values are non-negative? (can be a follow up)

wendigo · 2025-08-06T06:21:59Z

core/trino-main/src/main/java/io/trino/execution/resourcegroups/ResourceUsage.java

+    private final long physicalInputDataUsageBytes;

-    public ResourceUsage(long cpuUsageMillis, long memoryUsageBytes)
+    public ResourceUsage(long cpuUsageMillis, long memoryUsageBytes, long physicalInputDataUsageBytes)


this could be converted to record class (can be a follow up)

cla-bot bot added the cla-signed label Feb 21, 2025

github-actions bot added the docs label Feb 21, 2025

kekwan force-pushed the data-scan-resource-groups branch 5 times, most recently from a9ca3a5 to 8841fac Compare February 22, 2025 01:13

kekwan marked this pull request as ready for review February 22, 2025 01:13

kekwan changed the title ~~[WIP] Add physical data scan tracking to resource groups~~ Add physical data scan tracking to resource groups Feb 22, 2025

kekwan force-pushed the data-scan-resource-groups branch 2 times, most recently from 6b533d3 to bc39466 Compare February 22, 2025 01:34

wendigo requested review from hashhar, lukasz-stec and piotrrzysko and removed request for lukasz-stec February 22, 2025 16:03

kekwan force-pushed the data-scan-resource-groups branch 2 times, most recently from fab19d4 to 2074032 Compare March 14, 2025 16:33

kekwan force-pushed the data-scan-resource-groups branch 2 times, most recently from 9c548c3 to 819ddd9 Compare March 24, 2025 21:54

kekwan requested a review from wendigo April 12, 2025 00:53

kekwan force-pushed the data-scan-resource-groups branch from 819ddd9 to 7948bc4 Compare April 25, 2025 22:30

github-actions bot added the stale label May 19, 2025

github-actions bot closed this Jun 9, 2025

kekwan reopened this Jun 9, 2025

lukasz-stec reviewed Jun 10, 2025

View reviewed changes

github-actions bot removed the stale label Jun 10, 2025

kekwan force-pushed the data-scan-resource-groups branch 3 times, most recently from df31081 to 24e79c7 Compare July 2, 2025 20:45

kekwan force-pushed the data-scan-resource-groups branch 5 times, most recently from 448d088 to 27d4fab Compare July 24, 2025 20:55

kekwan requested a review from lukasz-stec July 24, 2025 20:55

kekwan commented Jul 24, 2025

View reviewed changes

kekwan requested a review from xkrogen July 24, 2025 23:02

lukasz-stec reviewed Jul 25, 2025

View reviewed changes

xkrogen reviewed Jul 25, 2025

View reviewed changes

kekwan force-pushed the data-scan-resource-groups branch from 5c811c5 to 1509517 Compare July 25, 2025 19:16

kekwan requested review from lukasz-stec and xkrogen July 28, 2025 20:52

lukasz-stec approved these changes Aug 5, 2025

View reviewed changes

...rs/src/main/java/io/trino/plugin/resourcegroups/db/ResourceGroupGlobalPropertiesReducer.java Outdated Show resolved Hide resolved

Add physical data scan tracking to resource groups

2a9d9dd

Add physical data scan tracking to resource groups. This is an optional feature in resource groups that tracks data scan via quota based tracking.

kekwan force-pushed the data-scan-resource-groups branch from 1509517 to 2a9d9dd Compare August 6, 2025 01:40

wendigo approved these changes Aug 6, 2025

View reviewed changes

wendigo reviewed Aug 6, 2025

View reviewed changes

wendigo merged commit 0a39998 into trinodb:master Aug 6, 2025
98 checks passed

github-actions bot added this to the 477 milestone Aug 6, 2025

ebyhr mentioned this pull request Aug 10, 2025

Add Trino 477 release notes #26350

Merged

Add physical data scan tracking to resource groups #25113

Add physical data scan tracking to resource groups #25113

Uh oh!

Conversation

kekwan commented Feb 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Additional context and related issues

Release notes

Uh oh!

kekwan commented Mar 5, 2025

Uh oh!

github-actions bot commented May 19, 2025

Uh oh!

github-actions bot commented Jun 9, 2025

Uh oh!

kekwan commented Jun 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kekwan Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kekwan commented Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lukasz-stec left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lukasz-stec Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

kekwan Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

xkrogen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kekwan commented Jul 25, 2025

Uh oh!

lukasz-stec left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wendigo Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

wendigo Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

kekwan commented Feb 21, 2025 •

edited

Loading

kekwan Jul 24, 2025 •

edited

Loading

kekwan commented Jul 24, 2025 •

edited

Loading

kekwan Jul 25, 2025 •

edited

Loading