Runless events - refactor job_versions_io_mapping

[pawel-big-lebowski]

Problem

This is currently a draft PR which is far from being merged. It is missing few tests related to schema changes which are marked with todo within the code. I've created a PR to have a better discussion on adding job_id to job_versions_io_mapping. This PR should be a follow-up of #2641.

The assumption was that it should be helpful in optimising get-lineage query. I would like first to clarify how are we going to make benefit of this extra column.

Solution

Please describe your change as it relates to the problem, or bug fix, as well as any dependencies. If your change requires a database schema migration, please describe the schema modification(s) and whether it's a backwards-incompatible or backwards-compatible change.

Note: All database schema changes require discussion. Please link the issue for context.

One-line summary:

Checklist

• You've signed-off your work
• Your changes are accompanied by tests (if relevant)
• Your change contains a small diff and is self-contained
• You've updated any relevant documentation (if relevant)
• You've included a one-line summary of your change for the CHANGELOG.md (Depending on the change, this may not be necessary).
• You've versioned your .sql database schema migration according to Flyway's naming convention (if relevant)
• You've included a header in any source code files (if relevant)

[netlify]

✅ Deploy Preview for peppy-sprite-186812 canceled.

Name	Link
🔨 Latest commit	a8cdbe0
🔍 Latest deploy log	https://app.netlify.com/sites/peppy-sprite-186812/deploys/65795f0ad206920009a4ac89

[codecov]

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (a5a0e55) 84.08% compared to head (a8cdbe0) 84.15%.

Files	Patch %	Lines
...rations/V67_2_JobVersionsIOMappingBackfillJob.java	81.81%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #2654      +/-   ##
============================================
+ Coverage     84.08%   84.15%   +0.06%     
- Complexity     1379     1390      +11     
============================================
  Files           248      249       +1     
  Lines          6295     6322      +27     
  Branches        286      286              
============================================
+ Hits           5293     5320      +27     
- Misses          849      850       +1     
+ Partials        153      152       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[wslulciuc]

Can we use DATASET or JOB as the lineageEventType? I think it would be helpful to know the event type.

[pawel-big-lebowski]

Currently, this column holds run states and its name is kind of misleading. Its name fits static lineage scenario well. However, I don't think we should store in a single column run-state and event-type. Renaming this column would require significant amount of work witch changes all over the project, including the spec.

Assuming this column contains run state update, then keeping it to null for DatasetEvent makes sense to me.

[wslulciuc]

I agree it's misleading, but Marquez doesn't necessarily have to adhere exactly to OpenLineage concepts and can redefine them. We can set lineage_event_type as null , but would prefer we (eventually) remap eventType to runState (just not within this PR).

[pawel-big-lebowski]

@wslulciuc I've added another commit to the PR keeping in mind upcoming streaming job support and our offline discussion.

Based on that, I think we should rename job_versions_io_mapping to job_io_mapping and make job_version_uuid nullable to be set at the end of the job.

[julienledem]

That's sounds fine to me. I'm curious to hear what Willy thinks.

[wslulciuc]

I like the idea of using a current version check, I'd be interesting to see the query plan and how the query may be optimized with the removal of join on that jobs table. Do we have any numbers on this?

[pawel-big-lebowski]

getLineage method is commonly used as it is entry point for a user to Marquez. The method is recursive and the purpose of this refactor is to make each recursion step to be computed within a single table with no joins required.

After this change, a whole lineage graph can be computed based on job_versions_io_mapping table. jobs_view is used only to enrich the returned graph nodes. Before this change, a join to jobs_view was needed in each recursion step to make sure if a row in job_versions_io_mapping represents current job version.

[wslulciuc]

Minor: symlink_target_job_uuid -> job_symlink_target_uuid since it's defined in the jobs table as jobs.symlink_target_uuid

[wslulciuc]

We set is_job_version_current = TRUE as a noop? i.e. just to fulfill the ON CONFLICT? Should we just use DO NOTHING instead?

[pawel-big-lebowski]

Yes, you're right we can go with do nothing.
markVersionIOMappingObsolete marks obsolete rows with job version different that a given one, so we don't need to implement conflict scenario here.

[wslulciuc]

Can we add a made_current_at column?

[wslulciuc]

Looking forward to the lineage query perf improvements and follow up analysis!