Uh oh!
There was an error while loading. Please reload this page.
SPARK-1126. spark-app preliminary - #86
Conversation
AmplabJenkins
commented
Mar 6, 2014
Merged build triggered. |
AmplabJenkins
commented
Mar 6, 2014
Merged build started. |
AmplabJenkins
commented
Mar 6, 2014
Merged build finished. |
AmplabJenkins
commented
Mar 6, 2014
One or more automated tests failed |
There was a problem hiding this comment.
I'd disable the scalastyle here so that it doesn't produce build errors. In this case I think it's fine to violate the line limit:
http://www.scalastyle.org/configuration.html
also there is a different way to do multline strings in scala - but up to you...
http://downgra.de/2010/09/14/multi-line_strings_with_scala/
mateiz
commented
Mar 6, 2014
Hey Sandy, the overall approach looks good, though I made some comments throughout. It would be really nice to avoid launching a second JVM if possible. It seems that the main reasons are to set environment vars or to pass arguments to the YARN launcher, but we can call the YARN launcher directly. |
mateiz
commented
Mar 6, 2014
Also, not sure what people think about calling this "spark-submit" instead of "spark-app". For the in-cluster use case it's really just for submitting, and I imagine that case will be more popular over time. |
sryza
commented
Mar 6, 2014
Thanks for taking a look, Matei. If we use system properties instead of env variables, the remaining reason we'd want to start a second JVM is to be able to have a --driver-memory property. The only way around this I can think of would be to require users to set this with an environment variable instead of a command line option. One small weird thing about this is that the client would still be given the max heap specified in driver SPARK_DRIVER_MEMORY even when the driver is being run on the cluster. |
sryza
commented
Mar 6, 2014
I uploaded a new patch that takes most of the review feedback into account. Includes the following changes:
I still need to tidy up the usage string. And there's the outstanding question of whether we can avoid starting a new JVM. |
AmplabJenkins
commented
Mar 6, 2014
Merged build triggered. |
AmplabJenkins
commented
Mar 6, 2014
Merged build started. |
AmplabJenkins
commented
Mar 6, 2014
Merged build finished. |
AmplabJenkins
commented
Mar 6, 2014
One or more automated tests failed |
mateiz
commented
Mar 6, 2014
I see, regarding the memory part, it sounds like we could do it in bash, but it might be kind of painful. We could do the following:
I agree that we shouldn't use the full memory you required if you submitted to a cluster. I'm not sure how hard it is to parse these arguments in bash -- it shouldn't be that hard, but we'll also have to do it in .cmd scripts on Windows and such. Otherwise It would be good to test how slow this is with two JVM launches (maybe we can avoid a lot of the slowness). |
sryza
commented
Mar 7, 2014
I uploaded a new patch that doesn't start a new JVM and parses --driver-memory in bash. It wasn't as bad as I expected (thanks to some help from @umbrant and @atm). I've verified that it works with yarn with both deploy modes. I'm still planning to add some tests and doc, but I wanted to upload it with the new approach in case there are any comments. |
AmplabJenkins
commented
Mar 7, 2014
Merged build triggered. |
AmplabJenkins
commented
Mar 7, 2014
Merged build started. |
There was a problem hiding this comment.
Are we envisioning a corresponding .cmd file once the review of this is done ?
There was a problem hiding this comment.
Yeah, though I think as a separate JIRA.
AmplabJenkins
commented
Mar 7, 2014
Merged build finished. |
AmplabJenkins
commented
Mar 7, 2014
One or more automated tests failed |
There was a problem hiding this comment.
would it make sense to say <application jar> instead of primary binary?
There was a problem hiding this comment.
The thinking behind "primary binary" was that we might support binaries that aren't jars for non-Java apps.
AmplabJenkins
commented
Mar 26, 2014
Merged build finished. |
AmplabJenkins
commented
Mar 26, 2014
All automated tests passed. |
pwendell
commented
Mar 26, 2014
Some surface-level comments, but looking pretty good. Will try to test on a standalone cluster later tonight. |
sryza
commented
Mar 26, 2014
Updated patch addresses Patrick's comments. |
AmplabJenkins
commented
Mar 26, 2014
Merged build triggered. |
AmplabJenkins
commented
Mar 26, 2014
Merged build started. |
AmplabJenkins
commented
Mar 26, 2014
Merged build finished. |
AmplabJenkins
commented
Mar 26, 2014
All automated tests passed. |
There was a problem hiding this comment.
It might be useful to give an example of actual usage of running one of the examples
tgravescs
commented
Mar 26, 2014
if I just run ./bin/spark-submit the usage has < primary binary > and the documentation (cluster-overview.md) seems to have < jar >. Should be make that the same? Also its not clear to me if I want to run one of the examples (SparkPi) on yarn should the primary binary be the examples jar or the spark jar itself? Perhaps just an examples would help with this or explaining what a primary binary is. Note I haven't looked at the code in detail so this is just from a users point of view. I'll dig into the class missing error to figure out what I was doing wrong. |
tgravescs
commented
Mar 26, 2014
Looks like the issues with missing Client class is due to https://spark-project.atlassian.net/browse/SPARK-1330 not this pr. Once that was fixed I am able to run both cluster and client mode on yarn Another thing I noticed is that the spark-submit script uses --arg and the spark-class script uses --args. Not a big deal just want to make sure we want arg vs args. I don't have a strong opinion on it but if people are used to using spark-class its just a change. It is a bit unfortunate we still have to specify the first arg as yarn-client or yarn-cluster for the spark examples so it can pass it to SparkContext but I guess there isn't much we can do about that since if it was real user code, the user could have that hardcoded or put it as any argument (not just the first one). Great work Sandy! Its nice to have this easier interface. |
sryza
commented
Mar 26, 2014
Thanks for the feedback, Tom. Regarding "primary binary" and "jar", to clear up confusion I'm just going to call it "app jar" for now and if/when we add support for non-jar binaries we can find something more suitable. Regarding arg vs. args, I found the plural in args confusing - it makes it seem like the parameter should take multiple values when in fact it takes a single value and can be specified multiple times. Other parameters with plurals, like "jars", don't work this way. We could possibly add --arg and deprecate --args for the spark-class way? |
tgravescs
commented
Mar 27, 2014
I guess its actually the yarn ClientArguments that takes the --args, not spark-class directly. I would be in favor of adding --arg and deprecating --args. With the spark-submit script I expect it to be hidden from most people going forward anyway. |
pwendell
commented
Mar 27, 2014
I'm also +1 moving to |
AmplabJenkins
commented
Mar 28, 2014
Can one of the admins verify this patch? |
pwendell
commented
Mar 29, 2014
Hey @sryza I'm going to submit a PR with some suggested follow-on changes, but I think we can go ahead and merge this for now as a starting point. Thanks for your work on this! |
This is a starting version of the spark-app script for running compiled binaries against Spark. It still needs tests and some polish. The only testing I've done so far has been using it to launch jobs in yarn-standalone mode against a pseudo-distributed cluster. This leaves out the changes required for launching python scripts. I think it might be best to save those for another JIRA/PR (while keeping to the design so that they won't require backwards-incompatible changes). Author: Sandy Ryza <sandy@cloudera.com> Closesapache#86 from sryza/sandy-spark-1126 and squashes the following commits: d428d85 [Sandy Ryza] Commenting, doc, and import fixes from Patrick's comments e7315c6 [Sandy Ryza] Fix failing tests 34de899 [Sandy Ryza] Change --more-jars to --jars and fix docs 299ddca [Sandy Ryza] Fix scalastyle a94c627 [Sandy Ryza] Add newline at end of SparkSubmit 04bc4e2 [Sandy Ryza] SPARK-1126. spark-submit script
…pache#86) * Check for user jars/files existence before creating the driver pod. Closeapache-spark-on-k8s#85 * CR
…pache#86) * Check for user jars/files existence before creating the driver pod. Closeapache-spark-on-k8s#85 * CR
…pache#86) * Check for user jars/files existence before creating the driver pod. Closeapache-spark-on-k8s#85 * CR
Added Hive support, as well as SparkR
…p table (apache#86) ### What changes were proposed in this pull request? Adds **Item 10, "The calendar as a lookup table, sized to the era"** to `SCOPE_MILESTONE_5.md`. A catalogue entry only — no code, and deliberately not a plan. It lands in milestone 5's catalogue rather than milestone 4's because milestone 4's file has become a task plan, and `sql/varka/AGENTS.md` sends work with no owner yet to the furthest-out scope document. By topic it belongs to the calendar family, and the item says so. **The idea.** ClickHouse's `DATE_LUT_SIZE` is `0x23AB1` — 146097, exactly one Gregorian era. It anchors that window at 1900 and falls back outside it, but 400 years is the calendar's *period*, so a table indexed by **day of era** needs no fallback for any `int32` date at all: every day reduces into it, and the year is `400 * (era - bias) + table[dayOfEra]`. Varka's prefix already computes that index — `emitEra` is the first thing it emits — so the table would replace everything after it. ClickHouse stores 16 bytes per day (year, month, day of month, day of week, days in month), so **one lookup yields every field**. That is the problem task 32 solves with a shared prefix, solved with memory instead, and it is the version of the idea worth measuring rather than the year-only one. **Already measured** (apache#85, `VarkaVectorApiProbeBenchmark`): with the column in a `MemorySegment` the way a real kernel has it, `year(d) = 1998` counted, an era-indexed year table runs at 2070.8 M rows/s against the arithmetic's 1329.3 — **1.6x** — at 571 KB for the table and about 10 KB touched by a seven-year query. ### Why are the changes needed? A 1.6x on a calendar node is worth writing down, and an idea with a measurement attached is the form this project's registers are supposed to take. Without an entry it becomes folklore from a conversation. The item is written so it can be **dropped as easily as built**: - It lists the five things that have to be measured before it is a task, led by the one comparison that actually matters and has *not* been run — a single gather yielding four fields against task 32's shared prefix at 797.7 M rows/s. Measuring only the year measures the weaker half of the idea. - It states the case against itself: the arithmetic is branch-free, needs no memory, and keeps getting cheaper — task 48 took four ops off the year tail, and apache#83 takes eighteen off `add_months` — while a table's cost is fixed and paid in cache. - It names the real cost if it proceeds: the emitter would gain a class of operand it has no notion of today, a constant table, and `GROUP_BUDGET` has no weight for a gather. It is deliberately **absent from section 5's ordering table**, with a sentence saying why: gated on that measurement, and not part of this milestone's decimal-and-aggregation spine. If the four-field number comes back the way the single-field one did, it becomes its own task and is scheduled then; if it does not, the entry is the record of why the idea was dropped, which is worth as much. ### Does this PR introduce _any_ user-facing change? No. One scope document; no code. ### How was this patch tested? Nothing to run. The numbers it cites are apache#85's committed benchmark rows rather than restated from a conversation, and the claim about what the gather can reach is the corrected one from that PR — an on-heap constant table indexed by off-heap data, not a gather from off-heap memory, which remains blocked. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5)
This is a starting version of the spark-app script for running compiled binaries against Spark. It still needs tests and some polish. The only testing I've done so far has been using it to launch jobs in yarn-standalone mode against a pseudo-distributed cluster.
This leaves out the changes required for launching python scripts. I think it might be best to save those for another JIRA/PR (while keeping to the design so that they won't require backwards-incompatible changes).