-
Notifications
You must be signed in to change notification settings - Fork 9.1k
YARN-11704. No need to apply extra 'AND' placement constraint for non tags in PlacementConstraintManager.getMultilevelConstraint #6940
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
Conversation
…cheduling request
PTAL @slfan1989 This is a small improvement for placement constraint mechanism |
💔 -1 overall
This message was automatically generated. |
mergedConstraint = (And) constraint.getConstraintExpr(); | ||
Assert.assertEquals(1, mergedConstraint.getChildren().size()); | ||
Assert.assertEquals(c1, mergedConstraint.getChildren().get(0).build()); | ||
Assert.assertTrue(constraint.getConstraintExpr().getClass() == c1.getConstraintExpr().getClass()); |
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.
We're closing this stale PR because it has been open for 100 days with no activity. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. |
Description of PR
After applying the placement constraint allocator in our internal cluster, I found some hotpoint that will effect the scheduling performance as the allocation time metric value increase a lot.
After dumping the flamegraph, the hotpoint is that the
SingleConstraintAppPlacementAllocator.checkCardinalityAndPending
, this should be optimized by like cache or other ways. But the unnecessary nested AND placement constraint may be a small improvement for this case.How was this patch tested?
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?