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.

Sign up for GitHub

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

Jump to bottom

[GOBBLIN-1725] Fix bugs in gaas warm standby mode #3582

Merged
merged 5 commits into from
Oct 18, 2022
Merged

[GOBBLIN-1725] Fix bugs in gaas warm standby mode #3582

merged 5 commits into from
Oct 18, 2022

Conversation

ZihanLi58
Copy link
Contributor

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):
  1. fix issue when initialize dag manager
  2. fix issue that we only check quota when we see compile succeed and flow is not explain
  3. Fix the missing config in gitFlowGraphMonitor (WIP)

Tests

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

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"

@codecov-commenter
Copy link

codecov-commenter commented Oct 14, 2022 •
edited
Loading

Codecov Report

Merging #3582 (61b3eea) into master (fafd40b) will decrease coverage by 0.00%.
The diff coverage is 62.50%.

@@             Coverage Diff              @@
##             master    #3582      +/-   ##
============================================
- Coverage     46.87%   46.87%   -0.01%     
- Complexity    10640    10643       +3     
============================================
  Files          2113     2113              
  Lines         82798    82808      +10     
  Branches       9220     9222       +2     
============================================
+ Hits          38813    38817       +4     
- Misses        40425    40432       +7     
+ Partials       3560     3559       -1
Impacted Files Coverage Δ
...ervice/modules/flow/BaseFlowToJobSpecCompiler.java 55.55% <33.33%> (ø)
...blin/service/modules/orchestration/DagManager.java 82.12% <75.00%> (+0.03%) ⬆️
...obblin/service/monitoring/GitFlowGraphMonitor.java 78.78% <100.00%> (+0.66%) ⬆️
.../java/org/apache/gobblin/cluster/SleepingTask.java 39.39% <0.00%> (-6.07%) ⬇️
...pache/gobblin/iceberg/writer/GobblinMCEWriter.java 74.89% <0.00%> (-0.67%) ⬇️
.../org/apache/gobblin/cluster/GobblinTaskRunner.java 63.63% <0.00%> (-0.32%) ⬇️
...apache/gobblin/hive/writer/HiveMetadataWriter.java 0.00% <0.00%> (ø)
...odules/orchestration/InMemoryUserQuotaManager.java 70.86% <0.00%> (+0.78%) ⬆️
...lin/elasticsearch/writer/FutureCallbackHolder.java 62.85% <0.00%> (+1.42%) ⬆️

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

umustafi
umustafi approved these changes Oct 17, 2022
View reviewed changes
Copy link
Contributor

@umustafi umustafi left a comment

Choose a reason for hiding this comment

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

LGTM

Will-Lo
Will-Lo requested changes Oct 17, 2022
View reviewed changes
gobblin-service/src/main/java/org/apache/gobblin/service/monitoring/GitFlowGraphMonitor.java Outdated
@@ -69,6 +69,7 @@ public class GitFlowGraphMonitor extends GitMonitoringService implements FlowGra
.put(ConfigurationKeys.FLOWGRAPH_POLLING_INTERVAL, DEFAULT_GIT_FLOWGRAPH_MONITOR_POLLING_INTERVAL)
Copy link
Contributor

Choose a reason for hiding this comment

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

This line can be removed, sorry I missed it with the other RB

@ZihanLi58 ZihanLi58 merged commit 0936016 into apache:master Oct 18, 2022
phet pushed a commit to phet/gobblin that referenced this pull request Dec 3, 2022
@ZihanLi58 @phet
[GOBBLIN-1725] Fix bugs in gaas warm standby mode (apache#3582)
f73aa28 
* address comments

* use connectionmanager when httpclient is not cloesable

* [GOBBLIN-1725] Fix bugs in gaas warm standby mode

* fix missing config

* address comments

Co-authored-by: Zihan Li <[email protected]>
phet added a commit to phet/gobblin that referenced this pull request Dec 5, 2022
@phet
Merge remote-tracking branch 'upstream/master'
83f243f 
* upstream/master:
  move dataset handler code before cleaning up staging data (apache#3594)
  [GOBBLIN-1730] Include flow execution id when try to cancel/submit job using SimpleKafkaSpecProducer (apache#3588)
  [GOBBLIN-1734] make DestinationDatasetHandler work on streaming sources (apache#3592)
  give option to cancel helix workflow through Delete API (apache#3580)
  [GOBBLIN-1728] Fix YarnService incorrect container allocation behavior (apache#3586)
  Support multiple node types in shared flowgraph, fix logs (apache#3590)
  Search for dummy file in writer directory (apache#3589)
  Use root cause for checking if exception is transient (apache#3585)
  [GOBBLIN-1724] Support a shared flowgraph layout in GaaS (apache#3583)
  [GOBBLIN-1731] Enable HiveMetadataWriter to override table schema lit… (apache#3587)
  [GOBBLIN-1726] Avro 1.9 upgrade of Gobblin OSS (apache#3581)
  [GOBBLIN-1725] Fix bugs in gaas warm standby mode (apache#3582)
  [GOBBLIN-1718] Define DagActionStoreMonitor to listen for kill/resume… (apache#3572)
  Add log line for committing/retrieving watermarks in streaming (apache#3578)
  [GOBBLIN-1707] Enhance `IcebergDataset` to detect when files already at dest then proceed with only delta (apache#3575)
  Ignore AlreadyExistsException in hive writer (apache#3579)
  Fail GMIP container for known transient exceptions to avoid data loss (apache#3576)
  GOBBLIN-1715: Support vectorized row batch pooling (apache#3574)
  [GOBBLIN-1696] Implement file based flowgraph that detects changes to the underlying… (apache#3548)
  GOBBLIN-1719 Replace moveToTrash with moveToAppropriateTrash for hadoop trash (apache#3573)
  [GOBBLIN-1703] avoid double quota increase for adhoc flows (apache#3550)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Will-Lo Will-Lo Will-Lo approved these changes

@umustafi umustafi umustafi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ZihanLi58 @codecov-commenter @Will-Lo @umustafi