-
Notifications
You must be signed in to change notification settings - Fork 4.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HIVE-24145 Fix preemption issues in reducers and file sink operators #1485
Conversation
try { | ||
fs.delete(updaters[i].getUpdatedFilePath(), true); | ||
} catch (IOException e) { | ||
e.printStackTrace(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOG?
@@ -284,6 +285,11 @@ public Object process(Node nd, Stack<Node> stack, NodeProcessorCtx procCtx, | |||
// Create ReduceSink operator | |||
ReduceSinkOperator rsOp = getReduceSinkOp(partitionPositions, sortPositions, sortOrder, sortNullOrder, | |||
allRSCols, bucketColumns, numBuckets, fsParent, fsOp.getConf().getWriteType()); | |||
// we have to make sure not to reorder the child operators as it might cause weird behavior in the tasks at | |||
// the same level. when there is auto stats gather at the same level as another operation then it might | |||
// cause unnecessary preemption. Maintaining the order here to avoid such preemption and possible errors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Plz add TEZ-3296 as ref if possible.
8b391e4
to
5c188e9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. +1 pending tests.
5c188e9
to
17d4660
Compare
17d4660
to
7bb23e7
Compare
7bb23e7
to
8a7e0c3
Compare
8a7e0c3
to
9b1169c
Compare
9b1169c
to
41e1b0b
Compare
What changes were proposed in this pull request?
Why are the changes needed?
Does this PR introduce any user-facing change?
How was this patch tested?