Skip to content
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

Don't log an ERROR for skipped scheduled executions #2000

Conversation

juhoautio
Copy link
Collaborator

@juhoautio juhoautio commented Oct 23, 2018

If a flow is scheduled with concurrentOption=skip, it's perfectly normal that triggering of a schedule is skipped. This PR changes such ERROR lines in the server log to INFO level.

On a general level, in my opinion ERROR level should be only used for platform errors, ie. when Azkaban fails to do something that it promises to be able to do. If this rule holds, it will be easier to monitor that Azkaban is working correctly by checking that server logs don't contain errors.

Before:

2018/10/23 13:41:14.337 +0300 INFO [ExecuteFlowAction] Invoking flow test-project.test-flow
2018/10/23 13:41:14.338 +0300 ERROR [TriggerManager] Failed to do action Execute flow test-flow from project test-project for Trigger Id: 0, Description: Trigger from triggerLoader with trigger condition of ThresholdChecker.eval() and expire condition of EndTimeCheck_1.eval(), Execute flow test-flow from project test-project
java.lang.RuntimeException: azkaban.executor.ExecutorManagerException: Flow is already running. Skipping execution.
	at azkaban.trigger.builtin.ExecuteFlowAction.doAction(ExecuteFlowAction.java:232)
	at azkaban.trigger.TriggerManager$TriggerScannerThread.onTriggerTrigger(TriggerManager.java:363)
	at azkaban.trigger.TriggerManager$TriggerScannerThread.checkAllTriggers(TriggerManager.java:343)
	at azkaban.trigger.TriggerManager$TriggerScannerThread.run(TriggerManager.java:297)
Caused by: azkaban.executor.ExecutorManagerException: Flow is already running. Skipping execution.
	at azkaban.trigger.builtin.ExecuteFlowAction.doAction(ExecuteFlowAction.java:229)
	... 3 more

After:

2018/10/23 13:41:51.778 +0300 INFO [ExecuteFlowAction] Invoking flow test-project.test-flow
2018/10/23 13:41:51.779 +0300 INFO [TriggerManager] Skipped action [Execute flow test-flow from project test-project] for [Trigger Id: 0, Description: Trigger from triggerLoader with trigger condition of ThresholdChecker.eval() and expire condition of EndTimeCheck_1.eval(), Execute flow test-flow from project test-project] because: Flow is already running. Skipping execution.

@codecov
Copy link

codecov bot commented Oct 23, 2018

Codecov Report

Merging #2000 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2000      +/-   ##
============================================
- Coverage     33.09%   33.08%   -0.01%     
+ Complexity     2690     2689       -1     
============================================
  Files           415      415              
  Lines         29280    29280              
  Branches       3714     3715       +1     
============================================
- Hits           9689     9688       -1     
  Misses        18724    18724              
- Partials        867      868       +1
Impacted Files Coverage Δ Complexity Δ
...ava/azkaban/trigger/builtin/ExecuteFlowAction.java 44% <0%> (+1.28%) 12 <0> (ø) ⬇️
.../src/main/java/azkaban/trigger/TriggerManager.java 15.31% <0%> (-0.2%) 5 <0> (ø)
...kaban/executor/RunningExecutionsUpdaterThread.java 75.86% <0%> (-3.45%) 6% <0%> (-1%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee6e229...632e43a. Read the comment docs.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 4148

  • 0 of 9 (0.0%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage remained the same at 36.169%

Changes Missing Coverage Covered Lines Changed/Added Lines %
azkaban-common/src/main/java/azkaban/trigger/builtin/ExecuteFlowAction.java 0 3 0.0%
azkaban-common/src/main/java/azkaban/trigger/TriggerManager.java 0 6 0.0%
Files with Coverage Reduction New Missed Lines %
azkaban-common/src/main/java/azkaban/trigger/builtin/ExecuteFlowAction.java 1 49.0%
Totals Coverage Status
Change from base Build 4147: 0.0%
Covered Lines: 10539
Relevant Lines: 29138

💛 - Coveralls

Copy link
Contributor

@burgerkingeater burgerkingeater left a comment

Choose a reason for hiding this comment

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

looks good! thanks

@burgerkingeater burgerkingeater merged commit 69d2de2 into azkaban:master Nov 2, 2018
@juhoautio juhoautio deleted the dont_log_error_for_skipped_scheduled_executions branch January 11, 2019 19:00
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.

None yet

3 participants