Repository navigation
SPARK-2127: Use application specific folders to dump metrics via CsvSink - #1067
Closed
rahulsinghaliitd wants to merge 1 commit into
Closed
rahulsinghaliitd wants to merge 1 commit into
rahulsinghaliitd wants to merge 1 commit into
Conversation
|
Can one of the admins verify this patch? |
Contributor
There was a problem hiding this comment.
Hi @rahulsinghaliitd , do we really need to use SparkConf to get this unique app name? I think metrics system is driven by its own configuration system, here you involved SparkConf as another configuration system just only for CsvSink, this will give people the chance to bypass the original way and lose its controllability. I think it would be nice to modify the way to get appUniqueName (eg. get it from SparkEnv).
Author
There was a problem hiding this comment.
@jerryshao Thanks for the feedback! I was not aware of the fact the SparkEnv provides access to SparkConf. Will follow up with the suggested modification.
Contributor
There was a problem hiding this comment.
Forgot to mention that several sinks like CsvSink will be started in master and worker for standalone mode if configured, in this situation you may not get unique app name or SparkEnv (something created only when app started). So maybe you should consider this situation when there's no app name available.
Author
There was a problem hiding this comment.
Hi @jerryshao ,
- At the moment the Sinks are being created, SparkEnv has not been created. I may be able to modify the Properties being passed to this Sink or even get the SparkConf from SecurityManager. But neither of those approaches seems generic to me. For e.g. we will need hadoopConf if we wanted the csv directory to be on HDFS.
- Thanks for pointing out the problem with Master and Worker. I have for now added app names to these classes. Please let me know if you think adding null checks in CsvSink would also be useful.
1. Generate an unique app name which is used to create events and metric folders. 2. Add app name & unique app name to Standalone mode's Master and Worker. 3. Log the unique app directory that will be used by CsvSink.
|
Can one of the admins verify this patch? |
wangyum
pushed a commit
that referenced
this pull request
May 26, 2023
…n for Hadoop platform (#1067) * [CARMEL-6200] Add unified command to extract table lineage information for Hadoop platform * Add tests
MaxGekk
added a commit
to MaxGekk/spark
that referenced
this pull request
Oct 27, 2026
### What changes were proposed in this pull request? A plan for reading the fields of a `TIMESTAMP_NTZ` in Varka's kernels, `sql/varka/plans/m8/VARKA-340.md`: the date fields (`year`, `month`, `day`, `quarter`, `dayofyear`, `weekofyear`, `last_day`, `trunc` and their `extract` forms), `hour`, `minute`, `second`, the casts with `TIME`, and `make_timestamp_ntz(date, time)`. The milestone plan's row opens with its state, and row 339's plan gets a dated note on the helper the two rows share. The plan starts from a check of today's code and of Spark's own, with a file and line for each fact. The facts that shape it: - **The fields are total.** A `TIMESTAMP_NTZ` is a long of microseconds read at UTC, and every field is defined for every long, so a field kernel fuses or hands a batch to Spark's row engine, and never raises. Only `make_timestamp_ntz` - and the `TIME` to `TIMESTAMP_NTZ` cast, which Spark rewrites into it - can overflow the long. Where Spark drops digits it truncates. - **No new IR node.** The day is the exact floor division by a day that the `TIMESTAMP_NTZ` cast to `DATE` already uses and proves; the time of day is the remainder; the date fields go through the existing calendar decomposition at the int lane, so one kernel crosses from the long lane to the int lane. No emitted byte of an existing kernel moves. - **The guard.** The calendar nodes need their day inside the calendar's range with a year of room below it, because the week forms and a field over `trunc` shift the day down; a batch with a day outside goes to the row engine. Row 339's plan had planned a tighter guard for the same helper, under which `weekofyear` would decline; the dated note there moves both rows to this one. - **The current date.** `CAST(time AS TIMESTAMP_NTZ)` is rewritten with the query's date as a literal; the plan passes it as a runtime argument outside the kernel's cache key, so one compiled kernel serves every date, and a test runs the same query under two time zones 26 hours apart to prove each answers its own date. Two points the first draft asked are settled by the row text: `SECOND`'s decimal output belongs to the row that builds decimal outputs, and the faster Granlund-Montgomery division is recorded but not built, since JDK 25's Vector API has no 64-bit high multiply. A baseline benchmark lands first as its own pull request. ### Why are the changes needed? `TIMESTAMP_NTZ` columns are read by Varka since apache#1031, but extracting their fields still runs on Spark's row engine, and the date fields over them are pinned as declining. The plan sets out the design, the guard and the proofs before any code is written. ### Does this PR introduce _any_ user-facing change? No. Plan documents only. ### How was this patch tested? `dev/varka_plan_lint.py --check` and `dev/varka_quote_check.py` pass. An independent review checked every claim against the code and against Spark's expressions and `DateTimeUtils`, rechecked the division constants by exact arithmetic, and found the cross-plan gap with row 339 and a claim about `trunc` wider than the code, all corrected here. Test time: none. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5.5)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Generate an unique app name which is used to create events and metric folders.