Skip to content

Conversation

@xguo27
Copy link
Contributor

@xguo27 xguo27 commented Nov 10, 2015

No description provided.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather move it to DAGScheduler's constructor as that's where the message belongs.

@andrewor14
Copy link
Contributor

Meh, I don't really think this is worth doing. It doesn't add any value or fix anything.

@xguo27
Copy link
Contributor Author

xguo27 commented Nov 17, 2015

I agree it is trivial, just thought I could quickly add a log statement. If Jacek agrees, I can close those PR.

@jaceklaskowski
Copy link
Contributor

I don't agree with @andrewor14. It does add a value of being consistent with how Spark informs about its status - if it says "Stopping..." at INFO it should be corresponding "Starting..." at INFO. That was my initial goal. Consistency is the goal and the value here.

Why does Spark report about services being stopped at all? What's the rationale?

I would change it to DEBUG, but since all the other logs about starting services are at INFO, I'd leave it as is until someone reports it should be done.

@xguo27
Copy link
Contributor Author

xguo27 commented Nov 23, 2015

@andrewor14 What is your take on Jacek's comment? I don't think it's a bad idea to make it more consistent with a matching log message. Please let me know. Thx!

@andrewor14
Copy link
Contributor

I just don't see any value. It's obvious that the scheduler is starting / stopping. Not a big deal if we merge it, but I just can't imagine anyone finding this message useful.

@markhamstra
Copy link
Contributor

Beyond that, the message is actually somewhat misleading. The "Stopping" message occurs in stop(), which is responsible for stopping the messageScheduler, eventProcessLoop and taskScheduler. When the "Starting" message is logged, at least the eventProcessLoop and taskScheduler will already have been started, so the argument from symmetry for "Starting" because there is a "Stopping" doesn't hold even without considering that "Starting", ...logMessage, ...logMessage, ... isn't symmetrical to logMessage, ...logMessage, "Stopping" unless we can somehow run time both forwards and backwards. There is also the problem that the "Starting" message is only sent from the secondary constructors and not from the primary.

@rxin
Copy link
Contributor

rxin commented Nov 24, 2015

I think this message is complete useless. I'd just remove it rather than spending time bickering about the wording.

@xguo27
Copy link
Contributor Author

xguo27 commented Nov 24, 2015

OK, I will close it. Thanks!

@xguo27 xguo27 closed this Nov 24, 2015
@xguo27 xguo27 deleted the SPARK-11631 branch November 24, 2015 08:12
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.

6 participants