Skip to content

YARN-6151. FS Preemption doesn't filter out queues which cannot be pr… #188

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.

Sign up for GitHub

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

Jump to bottom
Closed
flyrain wants to merge 3 commits into from
Closed

YARN-6151. FS Preemption doesn't filter out queues which cannot be pr… #188

flyrain wants to merge 3 commits into from

Conversation

@flyrain
Copy link
Contributor

@flyrain flyrain commented Feb 6, 2017

…eempted.

kambatla
kambatla reviewed Feb 7, 2017
View reviewed changes
...ager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FSQueue.java
*
* @return true if the queue can be preempted
*/
public boolean canBePreempted() {
Copy link
Contributor

@kambatla kambatla Feb 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this necessary?

...est/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestFairScheduler.java
assertEquals(3, scheduler.getSchedulerApp(app1).getPreemptionContainers()
assertEquals(4, scheduler.getSchedulerApp(app1).getPreemptionContainers()
.size());
}
Copy link
Contributor

@kambatla kambatla Feb 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a new test that verifies the exact scenario in the JIRA description?

...ager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FSQueue.java
* resource usage is greater than fair share.
*
* @return true if the queue can be preempted
*/
Copy link
Contributor

@kambatla kambatla Feb 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the check of the allowPreemptionFrom flag also be part of this method?

Copy link
Contributor Author

@flyrain flyrain Feb 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be, but allowPreemptionFrom is introduced after 2.8.x.

Copy link
Contributor

@kambatla kambatla Feb 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aah, I keep forgetting branch-2.8 was cut years ago. :(

kambatla
kambatla reviewed Feb 8, 2017
View reviewed changes
...est/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestFairScheduler.java Outdated
}

@Test
public void testPreemptionFilterOutNonPreemptableQueues() throws Exception {
Copy link
Contributor

@kambatla kambatla Feb 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add this test to TestFairSchedulerPreemption instead?

...ager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FSQueue.java Outdated
*/
public boolean canBePreempted() {
assert parent != null;
Preconditions.checkNotNull(parent, "Parent queue can't be null since"
Copy link
Contributor

@kambatla kambatla Feb 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, we could make this message more clear. "Parent queue is null. Looks like we are checking if root can be preempted."

Alternatively, can we make the if check (parent != null && ...)? That way, else would capture the null case and things should work fine?

@flyrain
Copy link
Contributor Author

flyrain commented Apr 4, 2017

Committed.

@flyrain flyrain closed this Apr 4, 2017
shanthoosh pushed a commit to shanthoosh/hadoop that referenced this pull request Oct 15, 2019
SAMZA-1288: Add null check for sink OutputStream
b31c0dc 
The logic to generate json for Sink operator does not check whether the output stream is null. This causes null pointer exception.

Author: Xinyu Liu <[email protected]>

Reviewers: Jake Maes <[email protected]>

Closes apache#188 from xinyuiscool/SAMZA-1288
p-szucs pushed a commit to p-szucs/hadoop that referenced this pull request Oct 22, 2025
@szetszwo @MikaelSmith
CDPD-90798: Use loadDefaults == true. (apache#188)
255b484 
(cherry picked from commit 53ffdb054c97217108a506fba4dea6476ed1703a)
Change-Id: Ie67b1968911c2f068c732c7cdd1f1bed0975f120
susheelgupta7 pushed a commit to susheel-gupta/hadoop that referenced this pull request Dec 22, 2025
Revert CDPD-90798. Backport FIPS SASL fixes to cdh_main. (apache#698)
9d3a4c0 
* CDPD-90798. Revert "CDPD-90798: Use loadDefaults == true. (apache#188)"
This reverts commit 255b484.

* CDPD-90798. Revert "CDPD-90798: Change back to env having a higher precedence than conf."
This reverts commit 90fe047.

* CDPD-90798. Revert "CDPD-90798: CDPD-76124.SaslMechanismFactory: Fixed typo in conf vs. env precedence logic"
This reverts commit 5496de1.

* CDPD-90798. Revert "CDPD-90798: HADOOP-19359. Accelerate token negotiation for other similar mechanisms."
This reverts commit 86f58d4.

* CDPD-90798. Revert "CDPD-90798: HDFS-17679 Use saslClient#hasInitialResponse() instead of heuristics in SaslParticipant#createFirstMessage() (apache#7201)"
This reverts commit bd0f557.

* CDPD-90798. Revert "CDPD-90798: HDFS-17668 Treat null SASL negotiated QOP as auth in DataTransferSasl… (apache#7171)"
This reverts commit 20eb4e5.

* CDPD-90798. Revert "CDPD-90798: HADOOP-19342. SaslRpcServer.AuthMethod print INFO messages in client."
This reverts commit 4c7ddb3.

* CDPD-90798. Revert "CDPD-90798: HADOOP-19306. Support user defined auth Callback in SaslRpcServer. (apache#7140)"
This reverts commit 1c0bd3d.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@kambatla kambatla kambatla approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@flyrain @kambatla