Skip to content

[SPARK-36543][CORE] Rearranged logging in decommission loop#35094

Closed
sungpeo wants to merge 1 commit intoapache:masterfrom
sungpeo:spark-36543
Closed

[SPARK-36543][CORE] Rearranged logging in decommission loop#35094
sungpeo wants to merge 1 commit intoapache:masterfrom
sungpeo:spark-36543

Conversation

@sungpeo
Copy link

@sungpeo sungpeo commented Jan 4, 2022

What changes were proposed in this pull request?

Lower logging level, and transferred logInfo in a loop to outside

Why are the changes needed?

[SPARK-36543] Decommission logs too frequent when waiting migration to finish

Does this PR introduce any user-facing change?

Yes, it changes the level of logs, and make it less frequent.

How was this patch tested?

It doesn't need

@github-actions github-actions bot added the CORE label Jan 4, 2022
@sungpeo
Copy link
Author

sungpeo commented Jan 4, 2022

@holdenk
Made it simply logging less.
I changed logInfo without changing states to logDebug.

@HyukjinKwon HyukjinKwon changed the title [SPARK-20624] rearranged logging in decommission loop [SPARK-36543][K8S] Rearranged logging in decommission loop Jan 4, 2022
@HyukjinKwon HyukjinKwon changed the title [SPARK-36543][K8S] Rearranged logging in decommission loop [SPARK-36543][CORE] Rearranged logging in decommission loop Jan 4, 2022
@HyukjinKwon
Copy link
Member

@sungpeo BTW, Apache Spark repository leverages the resources from GItHub Actions in your forked repository for your PR so it should be enabled in your forked repository to run the tests, see also https://github.com/apache/spark/pull/35094/checks?check_run_id=4699635992.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@sungpeo
Copy link
Author

sungpeo commented Jan 5, 2022

@HyukjinKwon
Thank you for letting me know.

I enabled Github actions again, and sync (rebase) my branch to the latest master.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Sorry, but I'm -1 for this change. This is helpful to detect the decommissioners' misbehavior or some deadlock situation. Why do you need to suppress this, @sungpeo ?

@sungpeo
Copy link
Author

sungpeo commented Jan 6, 2022

@dongjoon-hyun
logInfo for other cases in the loop remain.
I think those (the remain logInfo) are enough to track the decommission.

Below logInfo(s) were repeated every loop for waiting done.
I agree with that was too much.

...
          logInfo("Checking to see if we can shutdown.")
...
                logInfo("No running tasks, checking migrations")
...
                  logInfo("All blocks not yet migrated.")

@github-actions
Copy link

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

@github-actions github-actions bot added the Stale label Apr 17, 2022
@github-actions github-actions bot closed this Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants