[GOBBLIN-1846] Validate Multi-active Scheduler with Logs #3707

umustafi · 2023-06-20T23:25:10Z

Adds logging at 3 critical points to ensure no scheduled events are missed
- when jobs are first rescheduled (or updated)
- each trigger of a new job
- when jobs are unscheduled

Dear Gobblin maintainers,

Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!

JIRA

My PR addresses the following Gobblin JIRA issues and references them in the PR title. For example, "[GOBBLIN-XXX] My Gobblin PR"
- https://issues.apache.org/jira/browse/GOBBLIN-1846

Description

Here are some details about my PR, including screenshots (if applicable):
Follow up to previous PR GOBBLIN-1837 to add validation via logging statements that we will query to ensure no scheduled trigger events are missed during host downtime. Also moves Flow specific logic from JobScheduler to GobblinServiceJobScheduler.

Tests

My PR adds the following unit tests OR does not need testing for this extremely good reason:
N/A to be added in the future but will be validated using these statements and local deploy

Commits

My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"

* Adds logging at 3 critical points to ensure no scheduled events are missed - when jobs are first rescheduled (or updated) - each trigger of a new job - when jobs are unscheduled

phet

couple Qs toward clarifying naming

gobblin-runtime/src/main/java/org/apache/gobblin/scheduler/JobScheduler.java

codecov-commenter · 2023-06-21T00:10:36Z

Codecov Report

Merging #3707 (4134631) into master (dd9f9c4) will decrease coverage by 0.01%.
The diff coverage is 42.30%.

@@             Coverage Diff              @@
##             master    #3707      +/-   ##
============================================
- Coverage     46.83%   46.82%   -0.01%     
+ Complexity    10802    10801       -1     
============================================
  Files          2141     2141              
  Lines         84410    84429      +19     
  Branches       9383     9383              
============================================
+ Hits          39530    39533       +3     
- Misses        41276    41294      +18     
+ Partials       3604     3602       -2

Impacted Files	Coverage Δ
...pache/gobblin/configuration/ConfigurationKeys.java	`0.00% <ø> (ø)`
...ava/org/apache/gobblin/scheduler/JobScheduler.java	`0.00% <0.00%> (ø)`
.../modules/scheduler/GobblinServiceJobScheduler.java	`70.08% <47.82%> (-1.10%)`	⬇️

... and 10 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

gobblin-runtime/src/main/java/org/apache/gobblin/scheduler/JobScheduler.java

...e/src/main/java/org/apache/gobblin/service/modules/scheduler/GobblinServiceJobScheduler.java

phet · 2023-06-22T01:19:07Z

...e/src/main/java/org/apache/gobblin/service/modules/scheduler/GobblinServiceJobScheduler.java

-                  properties.getProperty(ConfigurationKeys.JOB_NAME_KEY),
-                  properties.getProperty(ConfigurationKeys.JOB_GROUP_KEY));
+          _log.info(jobSchedulerTracePrefixBuilder(properties) + "Unscheduled Spec");


I definitely support the reuse between these... but also see the prefix builder using FLOW_GROUP_KEY, while the prior revision had used JOB_GROUP_KEY. can the former stand in for the latter... or was it incorrect previously to use JOB_GROUP_KEY?

Previously was incorrectly using JOB_GROUP_KEY, we should track at FLOW level

* upstream/master: Fix bug with total count watermark whitelist (apache#3724) [GOBBLIN-1858] Fix logs relating to multi-active lease arbiter (apache#3720) [GOBBLIN-1838] Introduce total count based completion watermark (apache#3701) Correct num of failures (apache#3722) [GOBBLIN- 1856] Add flow trigger handler leasing metrics (apache#3717) [GOBBLIN-1857] Add override flag to force generate a job execution id based on gobbl… (apache#3719) [GOBBLIN-1855] Metadata writer tests do not work in isolation after upgrading to Iceberg 1.2.0 (apache#3718) Remove unused ORC writer code (apache#3710) [GOBBLIN-1853] Reduce # of Hive calls during schema related updates (apache#3716) [GOBBLIN-1851] Unit tests for MysqlMultiActiveLeaseArbiter with Single Participant (apache#3715) [GOBBLIN-1848] Add tags to dagmanager metrics for extensibility (apache#3712) [GOBBLIN-1849] Add Flow Group & Name to Job Config for Job Scheduler (apache#3713) [GOBBLIN-1841] Move disabling of current live instances to the GobblinClusterManager startup (apache#3708) [GOBBLIN-1840] Helix Job scheduler should not try to replace running workflow if within configured time (apache#3704) [GOBBLIN-1847] Exceptions in the JobLauncher should try to delete the existing workflow if it is launched (apache#3711) [GOBBLIN-1842] Add timers to GobblinMCEWriter (apache#3703) [GOBBLIN-1844] Ignore workflows marked for deletion when calculating container count (apache#3709) [GOBBLIN-1846] Validate Multi-active Scheduler with Logs (apache#3707) [GOBBLIN-1845] Changes parallelstream to stream in DatasetsFinderFilteringDecorator to avoid classloader issues in spark (apache#3706) [GOBBLIN-1843] Utility for detecting non optional unions should convert dataset urn to hive compatible format (apache#3705) [GOBBLIN-1837] Implement multi-active, non blocking for leader host (apache#3700) [GOBBLIN-1835]Upgrade Iceberg Version from 0.11.1 to 1.2.0 (apache#3697) Update CHANGELOG to reflect changes in 0.17.0 Reserving 0.18.0 version for next release [GOBBLIN-1836] Ensuring Task Reliability: Handling Job Cancellation and Graceful Exits for Error-Free Completion (apache#3699) [GOBBLIN-1805] Check watermark for the most recent hour for quiet topics (apache#3698) [GOBBLIN-1825]Hive retention job should fail if deleting underlying files fail (apache#3687) [GOBBLIN-1823] Improving Container Calculation and Allocation Methodology (apache#3692) [GOBBLIN-1830] Improving Container Transition Tracking in Streaming Data Ingestion (apache#3693) [GOBBLIN-1833]Emit Completeness watermark information in snapshotCommitEvent (apache#3696)

[GOBBLIN-1846] Validate Multi-active Scheduler with Logs

992527b

* Adds logging at 3 critical points to ensure no scheduled events are missed - when jobs are first rescheduled (or updated) - each trigger of a new job - when jobs are unscheduled

phet reviewed Jun 20, 2023

View reviewed changes

gobblin-runtime/src/main/java/org/apache/gobblin/scheduler/JobScheduler.java Outdated Show resolved Hide resolved

gobblin-runtime/src/main/java/org/apache/gobblin/scheduler/JobScheduler.java Outdated Show resolved Hide resolved

Will-Lo requested changes Jun 21, 2023

View reviewed changes

gobblin-runtime/src/main/java/org/apache/gobblin/scheduler/JobScheduler.java Outdated Show resolved Hide resolved

gobblin-runtime/src/main/java/org/apache/gobblin/scheduler/JobScheduler.java Outdated Show resolved Hide resolved

move flow related logic to GobblinServiceJobScheduler

c7f3c84

umustafi force-pushed the umustafi/validationForMultiActiveScheduler branch from 84c3717 to c7f3c84 Compare June 21, 2023 19:34

phet reviewed Jun 21, 2023

View reviewed changes

rename and reuse util

1ced6fe

phet reviewed Jun 22, 2023

View reviewed changes

phet approved these changes Jun 22, 2023

View reviewed changes

Will-Lo approved these changes Jun 22, 2023

View reviewed changes

fix typo to pass tests

4134631

Will-Lo merged commit 5af6bca into apache:master Jun 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GOBBLIN-1846] Validate Multi-active Scheduler with Logs #3707

[GOBBLIN-1846] Validate Multi-active Scheduler with Logs #3707

umustafi commented Jun 20, 2023 •

edited

Loading

phet left a comment

codecov-commenter commented Jun 21, 2023 •

edited

Loading

phet Jun 22, 2023

umustafi Jun 22, 2023

[GOBBLIN-1846] Validate Multi-active Scheduler with Logs #3707

[GOBBLIN-1846] Validate Multi-active Scheduler with Logs #3707

Conversation

umustafi commented Jun 20, 2023 • edited Loading

JIRA

Description

Tests

Commits

phet left a comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 21, 2023 • edited Loading

Codecov Report

phet Jun 22, 2023

Choose a reason for hiding this comment

umustafi Jun 22, 2023

Choose a reason for hiding this comment

umustafi commented Jun 20, 2023 •

edited

Loading

codecov-commenter commented Jun 21, 2023 •

edited

Loading