-
Notifications
You must be signed in to change notification settings - Fork 28k
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.
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
[SPARK-47599][MLLIB] MLLib: Migrate logWarn with variables to structured logging framework #46527
Conversation
…red logging framework
Let's continue on this one :) |
Okay, let me check it first, 😄 |
@@ -451,8 +451,8 @@ class KMeans @Since("1.5.0") ( | |||
|
|||
private def trainWithBlock(dataset: Dataset[_], instr: Instrumentation) = { | |||
if (dataset.storageLevel != StorageLevel.NONE) { | |||
instr.logWarning(s"Input vectors will be blockified to blocks, and " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete unnecessary string interpreter s"..."
@@ -179,8 +179,8 @@ class LinearSVC @Since("2.2.0") ( | |||
maxBlockSizeInMB) | |||
|
|||
if (dataset.storageLevel != StorageLevel.NONE) { | |||
instr.logWarning(s"Input instances will be standardized, blockified to blocks, and " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete unnecessary string interpreter. s"..."
@@ -129,9 +130,9 @@ class StopWordsRemover @Since("1.5.0") (@Since("1.5.0") override val uid: String | |||
if (Locale.getAvailableLocales.contains(Locale.getDefault)) { | |||
Locale.getDefault | |||
} else { | |||
logWarning(s"Default locale set was [${Locale.getDefault.toString}]; however, it was " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toString
is redundant
in our scenario. Let's remove it.
Ready for it! |
s" If failure occurs try setting executor-memory ${memThreshold}m (or larger).") | ||
logWarning(log"${MDC(LogKeys.CLASS_NAME, className)}.save() was called, " + | ||
log"but it may fail because of too little executor memory " + | ||
log"(${MDC(LogKeys.EXECUTION_MEMORY_SIZE, sc.executorMemory)}m). If failure occurs, " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log"(${MDC(LogKeys.EXECUTION_MEMORY_SIZE, sc.executorMemory)}m). If failure occurs, " + | |
log"(${MDC(LogKeys.EXECUTOR_MEMORY_SIZE, sc.executorMemory)}m). If failure occurs, " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, Good catch!
Thanks, updated!
@panbingkun Thanks for the work. Merging to master. |
What changes were proposed in this pull request?
The pr aims to migrate
logWarn
in moduleMLLib
with variables tostructured logging framework
.Why are the changes needed?
To enhance Apache Spark's logging system by implementing structured logging.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Was this patch authored or co-authored using generative AI tooling?
No.