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

[GOBBLIN-1853] Reduce # of Hive calls during schema related updates in metadata registration #3716

Merged
merged 1 commit into from
Jul 18, 2023

Conversation

homatthew
Copy link
Contributor

@homatthew homatthew commented Jul 18, 2023

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

Description

  • Here are some details about my PR, including screenshots (if applicable):
    The HiveMetadataWriter and IcebergMetadataWriter make schema changes and need to make calls to Hive / HDFS per GMCE. This is an unnecessary hotspot because we can reference the schema cache beforehand and determine whether we actually need to make network call.

Also, there are cases where the HiveMetadataWriter can update the latest schema to be a null schema. We want to fail earlier, and fail the code when updating the latest schema map instead of failing later with a NPE. I've also changed the if statement from containsKey to checking if the value is null so that if this somehow happens in the future, we do not try to update the table with a null schema which would cause a NPE

Tests

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

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"

@homatthew homatthew force-pushed the mh-improve-hive-GOBBLIN-1853 branch from 2024fa0 to d7daa83 Compare July 18, 2023 20:50
@homatthew homatthew force-pushed the mh-improve-hive-GOBBLIN-1853 branch from d7daa83 to b8b5c87 Compare July 18, 2023 21:13
import com.google.common.io.Closer;
import com.google.common.util.concurrent.ListenableFuture;

import javax.annotation.Nullable;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

original import ordering did not comply with Apache Gobblin OSS style guide

Copy link
Contributor

@ZihanLi58 ZihanLi58 left a comment

Choose a reason for hiding this comment

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

+1 LGTM, thanks for the fix

@codecov-commenter
Copy link

codecov-commenter commented Jul 18, 2023

Codecov Report

Merging #3716 (b8b5c87) into master (2c2ffac) will increase coverage by 0.15%.
The diff coverage is 5.88%.

@@             Coverage Diff              @@
##             master    #3716      +/-   ##
============================================
+ Coverage     46.79%   46.95%   +0.15%     
- Complexity    10793    10821      +28     
============================================
  Files          2143     2143              
  Lines         84528    84600      +72     
  Branches       9392     9404      +12     
============================================
+ Hits          39557    39721     +164     
+ Misses        41360    41255     -105     
- Partials       3611     3624      +13     
Impacted Files Coverage Δ
...apache/gobblin/hive/writer/HiveMetadataWriter.java 0.00% <0.00%> (ø)
.../gobblin/iceberg/writer/IcebergMetadataWriter.java 70.45% <20.00%> (-0.49%) ⬇️

... and 9 files with indirect coverage changes

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

@ZihanLi58 ZihanLi58 merged commit c4a466b into apache:master Jul 18, 2023
6 checks passed
phet added a commit to phet/gobblin that referenced this pull request Aug 15, 2023
* 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)
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.

3 participants