Skip to content

Conversation

@ryan-williams
Copy link
Contributor

  • pass $ZINC_PORT to zinc status/shutdown commands
  • fix path check that sets $ZINC_INSTALL_FLAG, which was incorrectly
    causing zinc to be shutdown and restarted every time (with mismatched
    ports on those commands to boot)
  • pass -DzincPort=${ZINC_PORT} to maven, to use the correct zinc port
    when building

also:
- fix path check that sets ZINC_INSTALL_FLAG, which was incorrectly
  causing zinc to be shutdown and restarted every time (with incorrect
  ports on those commands)
- pass -DzincPort=${ZINC_PORT} to maven, to use the correct zinc port
  when building
@JoshRosen
Copy link
Contributor

/cc @pwendell given that this is related to #7878

@pwendell
Copy link
Contributor

pwendell commented Aug 4, 2015

LGTM - thanks for adding this. We've been manually working around this on jenkins (by passing -DzincPort, etc) but it's much nicer to have this handled properly. /cc @brennonyork as well.

@brennonyork
Copy link

Do we need to set a default zinc port or are we assuming that either:

  1. the zinc port defaults if you pass in -port ""
  2. the ZINC_PORT variable will always be filled

I don't know the default behavior so just want to make sure. Was thinking we could set the default port like ZINC_PORT=${ZINC_PORT:-<default-port>}. Necessary?

@pwendell
Copy link
Contributor

pwendell commented Aug 4, 2015

@brennonyork I think the zinc port is already set if none is provided:

https://github.com/ryan-williams/spark/blob/zinc-status/build/mvn#L113

@brennonyork
Copy link

Totally missed that, awesome! Just wanted to make sure! LGTM.

@SparkQA
Copy link

SparkQA commented Aug 5, 2015

Test build #39756 has finished for PR 7944 at commit 619c520.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • ParamDesc[Array[Double]]("thresholds", "Thresholds in multi-class classification" +
    • final val thresholds: DoubleArrayParam = new DoubleArrayParam(this, "thresholds", "Thresholds in multi-class classification to adjust the probability of predicting each class. Array must have length equal to the number of classes, with values >= 0. The class with largest value p/t is predicted, where p is the original probability of that class and t is the class' threshold.", (t: Array[Double]) => t.forall(_ >= 0))

@ryan-williams
Copy link
Contributor Author

Just catching up here, yea @pwendell the -DzincPort bit was kind of an afterthought but I guess is really the most useful thing here :) it's also not really covered by either JIRA but they're all such trivial changes that I'm not going to file it separately unless you say to

@pwendell
Copy link
Contributor

pwendell commented Aug 5, 2015

Nah - not a big enough thing to deal to create a new JIRA. Anyways this LGTM. @JoshRosen feel free to merge.

asfgit pushed a commit that referenced this pull request Aug 5, 2015
- pass `$ZINC_PORT` to zinc status/shutdown commands
- fix path check that sets `$ZINC_INSTALL_FLAG`, which was incorrectly
  causing zinc to be shutdown and restarted every time (with mismatched
  ports on those commands to boot)
- pass `-DzincPort=${ZINC_PORT}` to maven, to use the correct zinc port
  when building

Author: Ryan Williams <ryan.blake.williams@gmail.com>

Closes #7944 from ryan-williams/zinc-status and squashes the following commits:

619c520 [Ryan Williams] fix zinc status/shutdown commands

(cherry picked from commit e27a8c4)
Signed-off-by: Sean Owen <sowen@cloudera.com>
asfgit pushed a commit that referenced this pull request Aug 5, 2015
- pass `$ZINC_PORT` to zinc status/shutdown commands
- fix path check that sets `$ZINC_INSTALL_FLAG`, which was incorrectly
  causing zinc to be shutdown and restarted every time (with mismatched
  ports on those commands to boot)
- pass `-DzincPort=${ZINC_PORT}` to maven, to use the correct zinc port
  when building

Author: Ryan Williams <ryan.blake.williams@gmail.com>

Closes #7944 from ryan-williams/zinc-status and squashes the following commits:

619c520 [Ryan Williams] fix zinc status/shutdown commands

(cherry picked from commit e27a8c4)
Signed-off-by: Sean Owen <sowen@cloudera.com>
asfgit pushed a commit that referenced this pull request Aug 5, 2015
- pass `$ZINC_PORT` to zinc status/shutdown commands
- fix path check that sets `$ZINC_INSTALL_FLAG`, which was incorrectly
  causing zinc to be shutdown and restarted every time (with mismatched
  ports on those commands to boot)
- pass `-DzincPort=${ZINC_PORT}` to maven, to use the correct zinc port
  when building

Author: Ryan Williams <ryan.blake.williams@gmail.com>

Closes #7944 from ryan-williams/zinc-status and squashes the following commits:

619c520 [Ryan Williams] fix zinc status/shutdown commands

(cherry picked from commit e27a8c4)
Signed-off-by: Sean Owen <sowen@cloudera.com>
@asfgit asfgit closed this in e27a8c4 Aug 5, 2015
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