[GOBBLIN-1857] Add override flag to force generate a job execution id based on gobbl… #3719

Will-Lo · 2023-07-19T20:25:00Z

…in cluster system time

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-1857

Description

Here are some details about my PR, including screenshots (if applicable):
Job Execution ID is automatically inferred from the flow execution ID if the job was orchestrated by Gobblin-as-a-Service. However, this can lead to bugs in conjunction with other job keys such as earlyStop, since it would create multiple planningJobs and job IDs sent to Helix. These jobs would then have the same execution ID which is rejected by Helix.

When the user sets a configuration gobblin.cluster.job.useGeneratedJobExecutionIds are set, we want to force the Gobblin cluster to default to creating the job execution ID from its timestamp. This may make the job harder to track but earlyStop jobs are already difficult to track due to having multiple submitted Helix jobs

Tests

My PR adds the following unit tests OR does not need testing for this extremely good reason:
Unit tests

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"

…in cluster system time

codecov-commenter · 2023-07-19T20:39:25Z

Codecov Report

Merging #3719 (f73b1ee) into master (4a9dd53) will increase coverage by 2.50%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #3719      +/-   ##
============================================
+ Coverage     46.93%   49.44%   +2.50%     
+ Complexity    10808     9341    -1467     
============================================
  Files          2143     1761     -382     
  Lines         84545    68937   -15608     
  Branches       9390     7857    -1533     
============================================
- Hits          39683    34086    -5597     
+ Misses        41244    31688    -9556     
+ Partials       3618     3163     -455

Impacted Files	Coverage Δ
...bblin/cluster/GobblinClusterConfigurationKeys.java	`50.00% <ø> (ø)`
...a/org/apache/gobblin/cluster/HelixJobsMapping.java	`81.53% <100.00%> (+0.58%)`	⬆️

... and 388 files with indirect coverage changes

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

umustafi · 2023-07-19T22:36:00Z

gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinClusterConfigurationKeys.java

@@ -180,6 +180,10 @@ public class GobblinClusterConfigurationKeys {
  public static final String CANCEL_RUNNING_JOB_ON_DELETE = GOBBLIN_CLUSTER_PREFIX + "job.cancelRunningJobOnDelete";
  public static final String DEFAULT_CANCEL_RUNNING_JOB_ON_DELETE = "false";

+  // Job Execution ID for Helix jobs is inferred from Flow Execution IDs, but there are scenarios in earlyStop jobs where
+  // this behavior needs to be avoided due to concurrent planning and acutal jobs sharing the same execution ID


small typo actual

umustafi · 2023-07-19T22:37:53Z

gobblin-cluster/src/main/java/org/apache/gobblin/cluster/HelixJobsMapping.java

@@ -96,15 +96,17 @@ public HelixJobsMapping(Config sysConfig, URI fsUri, String rootDir) {
  }

  public static String createPlanningJobId (Properties jobPlanningProps) {
+    long planningJobId = PropertiesUtils.getPropAsBoolean(jobPlanningProps, GobblinClusterConfigurationKeys.USE_GENERATED_JOBEXECUTION_IDS, "false") ?
+        System.currentTimeMillis() : PropertiesUtils.getPropAsLong(jobPlanningProps, ConfigurationKeys.FLOW_EXECUTION_ID_KEY, System.currentTimeMillis());


in non-earlyStop scenarios is there an advantage to re-using the flow execution id for job id? Is this just for identifying the job more easily? In the early stop case is there a log line or something we can use to track the job id or how do we make the association?

The advantage is primarily for tracking, GobblinTrackingEvents have an execution ID in their metadata and it would work nicely if the execution ID also allowed the caller to identify the flow execution ID from it as well, lets the user perform cancel() etc.
Uhh for earlyStop maybe we can append something to the jobName but it'll be hacky, I feel like that feature should be redesigned in general to follow gobblin conventions better

* 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)

Add override flag to force generate a job execution id based on gobbl…

3711276

…in cluster system time

umustafi suggested changes Jul 19, 2023

View reviewed changes

fix typo

f73b1ee

umustafi approved these changes Jul 20, 2023

View reviewed changes

Will-Lo merged commit bea009e into apache:master Jul 20, 2023
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GOBBLIN-1857] Add override flag to force generate a job execution id based on gobbl… #3719

[GOBBLIN-1857] Add override flag to force generate a job execution id based on gobbl… #3719

Will-Lo commented Jul 19, 2023 •

edited

Loading

codecov-commenter commented Jul 19, 2023 •

edited

Loading

umustafi Jul 19, 2023

umustafi Jul 19, 2023

Will-Lo Jul 20, 2023 •

edited

Loading

[GOBBLIN-1857] Add override flag to force generate a job execution id based on gobbl… #3719

[GOBBLIN-1857] Add override flag to force generate a job execution id based on gobbl… #3719

Conversation

Will-Lo commented Jul 19, 2023 • edited Loading

JIRA

Description

Tests

Commits

codecov-commenter commented Jul 19, 2023 • edited Loading

Codecov Report

umustafi Jul 19, 2023

Choose a reason for hiding this comment

umustafi Jul 19, 2023

Choose a reason for hiding this comment

Will-Lo Jul 20, 2023 • edited Loading

Choose a reason for hiding this comment

Will-Lo commented Jul 19, 2023 •

edited

Loading

codecov-commenter commented Jul 19, 2023 •

edited

Loading

Will-Lo Jul 20, 2023 •

edited

Loading