Skip to content

Fix cumulativeUserMemory initial calculation & use double for cumulativeUserMemory consistently#16026

Merged
arhimondr merged 1 commit intoprestodb:masterfrom
frankobe:frankhu/fix-cumulative-user-memory
Apr 30, 2021
Merged

Fix cumulativeUserMemory initial calculation & use double for cumulativeUserMemory consistently#16026
arhimondr merged 1 commit intoprestodb:masterfrom
frankobe:frankhu/fix-cumulative-user-memory

Conversation

@frankobe
Copy link
Contributor

@frankobe frankobe commented Apr 29, 2021

Currently there is a chance that Cumulative User Memory is much larger than Peak User Memory * Execution Time, refer to the following screenshot of one sample query:

image

Extra debug logging in TaskContext.java shows,

2021-04-29T18:37:01.352Z    INFO    Task-20210429_183659_00001_stagingva.9.0.16-288 com.facebook.presto.operator.TaskContext    Cumulate user memory: sinceLastPeriodMillis = (34262137327073190 - 0) / 
1_000_000 = 34262137327.073193 ,averageMemoryForLastPeriod= (3184992 + 0) / 2 = 1592496

The bug is that sinceLastPeriodMillis can equal to current system nano time if the lastTaskStatCallNanos is not initialized and defaults to 0.

The fix is basically initialize the lastTaskStatCallNanos to task start time.

Signed-off-by: frankobe mua08p@gmail.com

Test plan - (Please fill in how you tested your changes)

Unit Test

== RELEASE NOTES ==

General Changes
* Fix amplified `Cumulative User Memory` metrics calculation

@frankobe frankobe force-pushed the frankhu/fix-cumulative-user-memory branch from 629daef to 20a71dd Compare April 30, 2021 00:30
@mbasmanova mbasmanova requested a review from arhimondr April 30, 2021 01:38
…iveUserMemory consistently

Signed-off-by: frankobe <mua08p@gmail.com>
@frankobe frankobe force-pushed the frankhu/fix-cumulative-user-memory branch from 20a71dd to 6f7b1b3 Compare April 30, 2021 16:02
@frankobe
Copy link
Contributor Author

@arhimondr Thx 4 review. Would u mind merging it? I am not authorized to do so.

@arhimondr arhimondr merged commit ea52ee0 into prestodb:master Apr 30, 2021
@frankobe frankobe deleted the frankhu/fix-cumulative-user-memory branch April 30, 2021 17:51
@bhhari bhhari mentioned this pull request May 11, 2021
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants