Skip to content
This repository has been archived by the owner on Aug 4, 2022. It is now read-only.

[REEF-1207] REEF.NET Evaluator should print a known log line on exit #991

Closed
wants to merge 2 commits into from

Conversation

afchung
Copy link
Contributor

@afchung afchung commented May 5, 2016

This addressed the issue by

  • Adding constnats for the known Evaluator exit line.
  • Logging the lines on Evaluator Exit and ensuring that exits are only logged a single time.

JIRA:
REEF-1207

This addressed the issue by
  * Adding constnats for the known Evaluator exit line.
  * Logging the lines on Evaluator Exit and ensuring that exits are only logged a single time.

JIRA:
  [REEF-1207](https://issues.apache.org/jira/browse/REEF-1207)
{
}

public void LogExit(bool successfulExit)
Copy link
Contributor

Choose a reason for hiding this comment

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

internal

Copy link
Contributor

Choose a reason for hiding this comment

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

comment

@markusweimer
Copy link
Contributor

I've done a pass.

@jwang98052
Copy link
Contributor

I will look at it.

@@ -208,11 +211,11 @@ public void OnNext(RuntimeStop runtimeStop)
}
catch (Exception e)
{
Utilities.Diagnostics.Exceptions.CaughtAndThrow(new InvalidOperationException("Cannot stop evaluator properly", e), Level.Error, "Exception during shut down.", Logger);
_evaluatorExitLogger.LogExit(false);
Utilities.Diagnostics.Exceptions.CaughtAndThrow(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is EvaluatorExitLogger supposed to log the last line in evaluator, or it would want to replace current multiple unclear logs before evaluator exit? Here CaughtAndThrow will log again.

@jwang98052
Copy link
Contributor

I completed a pass.

@afchung
Copy link
Contributor Author

afchung commented May 9, 2016

@markusweimer @jwang98052 I've addressed your comments, please have another look. Thanks!

@jwang98052
Copy link
Contributor

LGTM

@markusweimer
Copy link
Contributor

LGTM, will tets and merge

@asfgit asfgit closed this in d0bc8df May 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants