Skip to content

Conversation

@windkit
Copy link
Contributor

@windkit windkit commented Oct 23, 2017

What changes were proposed in this pull request?

Documentation about Mesos Reject Offer Configurations

Related PR

#19510 for spark.mem.max

@skonto
Copy link
Contributor

skonto commented Oct 23, 2017

@susanxhuynh could you pls review this?

Copy link

@ArtRand ArtRand left a comment

Choose a reason for hiding this comment

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

I would prefer that the spark.mem.max changes be a part of #19510 only. Just some nits w.r.t wording. Thanks for doing this!

Copy link

Choose a reason for hiding this comment

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

Could you please add these changes only in #19510?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ArtRand Sure, I will move the documentation to 19510

Copy link

Choose a reason for hiding this comment

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

reserve is probably not the correct term to use here. I would use consume, as Spark does not actually make resource reservations http://mesos.apache.org/documentation/latest/reservation/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree. I will update it later on

Copy link

Choose a reason for hiding this comment

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

As above, please add this in the separate PR.

Copy link

Choose a reason for hiding this comment

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

This doesn't sound correct. The mesos.proto (https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L2310) states:

Time to consider unused resources refused. Note that all unused
resources will be considered refused and use the default value
(below) regardless of whether Filters was passed to
SchedulerDriver::launchTasks. You MUST pass Filters with this
field set to change this behavior (i.e., get another offer which
includes unused resources sooner or later than the default).

some simple word-smithing or a link should make it clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the comment, I will update it.

@ArtRand
Copy link

ArtRand commented Oct 24, 2017

LGTM

@ArtRand
Copy link

ArtRand commented Nov 8, 2017

Hey @srowen can we get a merge on this?

@SparkQA
Copy link

SparkQA commented Nov 8, 2017

Test build #3984 has finished for PR 19555 at commit 5f3b35b.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member

srowen commented Nov 8, 2017

Merged to master

@asfgit asfgit closed this in 6447d7b Nov 8, 2017
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.

5 participants