Skip to content

Commit c526fa0

Browse files
authored
misc: Make event loop THE implementation for httpRemoteTask and related code (#26697)
## Description 1. event loop based http remote task, task status fetcher and task info fetcher has been reliably running in production for many months now. This pr makes event loop as the default implementation for those three classes and remove the old code. 2. I also tried to replay production queries with only 5 threads for the event loop attempting to catch any potential issues but no issue has been found. 3. heapdump analysis also showed that the failTask call is heavy containing a recursive toFailure call. This is something we can optimize. ## Motivation and Context clean up the duplicate code back then for roll out purpose. ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan 1. passed verifiers ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. - [ ] If adding new dependencies, verified they have an [OpenSSF Scorecard](https://securityscorecards.dev/#the-checks) score of 5.0 or higher (or obtained explicit TSC approval for lower scores). ## Release Notes ``` == NO RELEASE NOTE == ```
1 parent 0741995 commit c526fa0

File tree

9 files changed

+33
-2774
lines changed

9 files changed

+33
-2774
lines changed

presto-main-base/src/main/java/com/facebook/presto/execution/TaskManagerConfig.java

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@ public class TaskManagerConfig
9999
private double highMemoryTaskKillerGCReclaimMemoryThreshold = 0.01;
100100
private Duration highMemoryTaskKillerFrequentFullGCDurationThreshold = new Duration(1, SECONDS);
101101
private double highMemoryTaskKillerHeapMemoryThreshold = 0.9;
102-
private boolean enableEventLoop = true;
103102
private Duration slowMethodThresholdOnEventLoop = new Duration(0, SECONDS);
104103

105104
public long getSlowMethodThresholdOnEventLoop()
@@ -114,18 +113,6 @@ public TaskManagerConfig setSlowMethodThresholdOnEventLoop(Duration slowMethodTh
114113
return this;
115114
}
116115

117-
@Config("task.enable-event-loop")
118-
public TaskManagerConfig setEventLoopEnabled(boolean enableEventLoop)
119-
{
120-
this.enableEventLoop = enableEventLoop;
121-
return this;
122-
}
123-
124-
public boolean isEventLoopEnabled()
125-
{
126-
return enableEventLoop;
127-
}
128-
129116
@MinDuration("1ms")
130117
@MaxDuration("10s")
131118
@NotNull

presto-main-base/src/test/java/com/facebook/presto/execution/TestTaskManagerConfig.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,7 @@ public void testDefaults()
8080
.setHighMemoryTaskKillerFrequentFullGCDurationThreshold(new Duration(1, SECONDS))
8181
.setHighMemoryTaskKillerHeapMemoryThreshold(0.9)
8282
.setTaskUpdateSizeTrackingEnabled(true)
83-
.setSlowMethodThresholdOnEventLoop(new Duration(0, SECONDS))
84-
.setEventLoopEnabled(true));
83+
.setSlowMethodThresholdOnEventLoop(new Duration(0, SECONDS)));
8584
}
8685

8786
@Test
@@ -130,7 +129,6 @@ public void testExplicitPropertyMappings()
130129
.put("experimental.task.high-memory-task-killer-frequent-full-gc-duration-threshold", "2s")
131130
.put("experimental.task.high-memory-task-killer-heap-memory-threshold", "0.8")
132131
.put("task.update-size-tracking-enabled", "false")
133-
.put("task.enable-event-loop", "false")
134132
.put("task.event-loop-slow-method-threshold", "10m")
135133
.build();
136134

@@ -177,7 +175,6 @@ public void testExplicitPropertyMappings()
177175
.setHighMemoryTaskKillerFrequentFullGCDurationThreshold(new Duration(2, SECONDS))
178176
.setHighMemoryTaskKillerHeapMemoryThreshold(0.8)
179177
.setTaskUpdateSizeTrackingEnabled(false)
180-
.setEventLoopEnabled(false)
181178
.setSlowMethodThresholdOnEventLoop(new Duration(10, MINUTES));
182179

183180
assertFullMapping(properties, expected);

presto-main/src/main/java/com/facebook/presto/server/remotetask/ContinuousTaskStatusFetcher.java

Lines changed: 0 additions & 312 deletions
This file was deleted.

0 commit comments

Comments
 (0)