Source Facebook Marketing: Attempt to retry failing jobs that are already split to minimum size #12390

Phlair · 2022-04-27T12:27:00Z

What

Haven't been able to replicate locally, but I believe that we are prematurely failing the sync if an async job is already split to the smallest size and then it fails. This is because we try to split the job again and raise a RuntimeError if it's the smallest size.

How

I've introduced a catch so that instead of raising an error, we .restart() async jobs that are already the smallest size and fail.
In order to avoid infinitely retrying, I've added a check in the async_job_manager to raise an exception if any nested jobs within a ParentAsyncJob have hit the MAX_ATTEMPTS number.

My hope is that the failures the customer is seeing are transient and by doing this we solve the problem. Even if not, I think this is a positive change to allow all jobs the same number of retries.

TODO: Dockerfile and Changelog, waiting on reviews.

Phlair · 2022-04-27T12:28:40Z

/test connector=connectors/source-facebook-marketing

🕑 connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/2233055130
✅ connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/2233055130
Python tests coverage:

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/config.py                        74      6    92%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
source_acceptance_test/utils/common.py                  80     17    79%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/tests/test_core.py              285    106    63%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/utils/connector_runner.py       110     48    56%
source_acceptance_test/tests/test_incremental.py        69     38    45%
------------------------------------------------------------------------
TOTAL                                                  896    259    71%
Name                                                        Stmts   Miss  Cover
-------------------------------------------------------------------------------
source_facebook_marketing/streams/__init__.py                   2      0   100%
source_facebook_marketing/spec.py                              33      0   100%
source_facebook_marketing/__init__.py                           2      0   100%
source_facebook_marketing/api.py                               96     12    88%
source_facebook_marketing/streams/base_streams.py             127     27    79%
source_facebook_marketing/streams/common.py                    41     13    68%
source_facebook_marketing/streams/streams.py                   97     32    67%
source_facebook_marketing/source.py                            39     16    59%
source_facebook_marketing/streams/base_insight_streams.py     129     54    58%
source_facebook_marketing/streams/async_job.py                213    136    36%
source_facebook_marketing/streams/async_job_manager.py         74     56    24%
-------------------------------------------------------------------------------
TOTAL                                                         853    346    59%
Name                                                        Stmts   Miss  Cover
-------------------------------------------------------------------------------
source_facebook_marketing/streams/async_job.py                213      0   100%
source_facebook_marketing/streams/__init__.py                   2      0   100%
source_facebook_marketing/spec.py                              33      0   100%
source_facebook_marketing/__init__.py                           2      0   100%
source_facebook_marketing/streams/common.py                    41      1    98%
source_facebook_marketing/source.py                            39      1    97%
source_facebook_marketing/streams/async_job_manager.py         74      3    96%
source_facebook_marketing/api.py                               96      9    91%
source_facebook_marketing/streams/base_insight_streams.py     129     13    90%
source_facebook_marketing/streams/streams.py                   97     22    77%
source_facebook_marketing/streams/base_streams.py             127     30    76%
-------------------------------------------------------------------------------
TOTAL                                                         853     79    91%

codecov · 2022-04-27T12:34:46Z

Codecov Report

❗ No coverage uploaded for pull request base (master@c570225). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #12390   +/-   ##
=========================================
  Coverage          ?   90.74%           
=========================================
  Files             ?       11           
  Lines             ?      854           
  Branches          ?        0           
=========================================
  Hits              ?      775           
  Misses            ?       79           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c570225...6650671. Read the comment docs.

sherifnada

if we have no other retry logic anywhere this makes sense

...grations/connectors/source-facebook-marketing/source_facebook_marketing/streams/async_job.py

sherifnada · 2022-04-27T17:21:48Z

...grations/connectors/source-facebook-marketing/source_facebook_marketing/streams/async_job.py

@@ -58,6 +58,9 @@ class Status(str, Enum):
 class AsyncJob(ABC):
    """Abstract AsyncJob base class"""

+    # max attempts for a job before errroring out
+    max_attempts: int = 10  # TODO: verify a sane number for this


am I reading this correctly that we currently have zero retry logic for async jobs, and that we just keep splitting until the job fails at the moment? if so this change makes a lot of sense

I'd recommend setting this to something like 5, it seems like if something fails 9 times it's unlikely to succeed on the 10th? (but you never know when it comes to FB :)

There is retry logic in async_job_manager.py but because we try to split_job() as soon as we hit attempt_number 2, we're ending up throwing this error of lowest split level only after retrying that call once (I think) rather than the specified 20 times.

I'm going to confirm that is what's happening and if so then refactor this PR so that rather than adding new retry logic in the AsyncJob it all ties together properly with the async_job_manager.

...grations/connectors/source-facebook-marketing/source_facebook_marketing/streams/async_job.py

Phlair · 2022-04-29T12:40:34Z

updated PR description to match new logic

Phlair · 2022-04-29T12:48:07Z

/test connector=connectors/source-facebook-marketing

🕑 connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/2245425599
✅ connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/2245425599
Python tests coverage:

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/config.py                        74      6    92%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
source_acceptance_test/utils/common.py                  80     17    79%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/tests/test_core.py              285    106    63%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/utils/connector_runner.py       110     48    56%
source_acceptance_test/tests/test_incremental.py        69     38    45%
------------------------------------------------------------------------
TOTAL                                                  896    259    71%
Name                                                        Stmts   Miss  Cover
-------------------------------------------------------------------------------
source_facebook_marketing/streams/__init__.py                   2      0   100%
source_facebook_marketing/spec.py                              33      0   100%
source_facebook_marketing/__init__.py                           2      0   100%
source_facebook_marketing/api.py                               96     12    88%
source_facebook_marketing/streams/base_streams.py             127     27    79%
source_facebook_marketing/streams/common.py                    41     13    68%
source_facebook_marketing/streams/streams.py                   97     32    67%
source_facebook_marketing/source.py                            39     16    59%
source_facebook_marketing/streams/base_insight_streams.py     129     54    58%
source_facebook_marketing/streams/async_job.py                210    134    36%
source_facebook_marketing/streams/async_job_manager.py         78     60    23%
-------------------------------------------------------------------------------
TOTAL                                                         854    348    59%
Name                                                        Stmts   Miss  Cover
-------------------------------------------------------------------------------
source_facebook_marketing/streams/async_job.py                210      0   100%
source_facebook_marketing/streams/__init__.py                   2      0   100%
source_facebook_marketing/spec.py                              33      0   100%
source_facebook_marketing/__init__.py                           2      0   100%
source_facebook_marketing/streams/common.py                    41      1    98%
source_facebook_marketing/source.py                            39      1    97%
source_facebook_marketing/streams/async_job_manager.py         78      3    96%
source_facebook_marketing/api.py                               96      9    91%
source_facebook_marketing/streams/base_insight_streams.py     129     13    90%
source_facebook_marketing/streams/streams.py                   97     22    77%
source_facebook_marketing/streams/base_streams.py             127     30    76%
-------------------------------------------------------------------------------
TOTAL                                                         854     79    91%

Phlair · 2022-05-03T10:07:46Z

/publish connector=connectors/source-facebook-marketing

🕑 connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/2263186851
🚀 Successfully published connectors/source-facebook-marketing
🚀 Auto-bumped version for connectors/source-facebook-marketing
✅ connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/2263186851

…eady split to minimum size (#12390) * restart jobs that are already split to smallest size * manager now fails on nested jobs hitting max attempts * version bump * auto-bump connector version Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>

restart jobs that are already split to smallest size

cfa3699

github-actions bot added the area/connectors Connector related issues label Apr 27, 2022

Phlair requested review from keu, girarda and sherifnada April 27, 2022 12:27

sherifnada removed the request for review from keu April 27, 2022 17:11

sherifnada reviewed Apr 27, 2022

View reviewed changes

Phlair added 2 commits April 29, 2022 11:57

Merge branch 'master' into george/fb-marketing

0fb5d03

manager now fails on nested jobs hitting max attempts

3cc9060

Phlair requested a review from sherifnada May 2, 2022 09:39

girarda approved these changes May 3, 2022

View reviewed changes

Phlair added 2 commits May 3, 2022 10:53

Merge branch 'master' into george/fb-marketing

5de126d

version bump

7db3e96

github-actions bot added the area/documentation Improvements or additions to documentation label May 3, 2022

auto-bump connector version

6650671

octavia-squidington-iii temporarily deployed to more-secrets May 3, 2022 10:30 Inactive

Phlair merged commit 9ffd5bb into master May 3, 2022

Phlair deleted the george/fb-marketing branch May 3, 2022 11:32

octavia-squidington-iii mentioned this pull request May 3, 2022

Bump Airbyte version from 0.36.6-alpha to 0.36.7-alpha #12535

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Source Facebook Marketing: Attempt to retry failing jobs that are already split to minimum size #12390

Source Facebook Marketing: Attempt to retry failing jobs that are already split to minimum size #12390

Phlair commented Apr 27, 2022 •

edited

Loading

Phlair commented Apr 27, 2022 •

edited by github-actions bot

Loading

codecov bot commented Apr 27, 2022 •

edited

Loading

sherifnada left a comment

sherifnada Apr 27, 2022

Phlair Apr 27, 2022 •

edited

Loading

Phlair commented Apr 29, 2022

Phlair commented Apr 29, 2022 •

edited by github-actions bot

Loading

Phlair commented May 3, 2022 •

edited by github-actions bot

Loading

Source Facebook Marketing: Attempt to retry failing jobs that are already split to minimum size #12390

Source Facebook Marketing: Attempt to retry failing jobs that are already split to minimum size #12390

Conversation

Phlair commented Apr 27, 2022 • edited Loading

What

How

Phlair commented Apr 27, 2022 • edited by github-actions bot Loading

codecov bot commented Apr 27, 2022 • edited Loading

Codecov Report

sherifnada left a comment

Choose a reason for hiding this comment

sherifnada Apr 27, 2022

Choose a reason for hiding this comment

Phlair Apr 27, 2022 • edited Loading

Choose a reason for hiding this comment

Phlair commented Apr 29, 2022

Phlair commented Apr 29, 2022 • edited by github-actions bot Loading

Phlair commented May 3, 2022 • edited by github-actions bot Loading

Phlair commented Apr 27, 2022 •

edited

Loading

Phlair commented Apr 27, 2022 •

edited by github-actions bot

Loading

codecov bot commented Apr 27, 2022 •

edited

Loading

Phlair Apr 27, 2022 •

edited

Loading

Phlair commented Apr 29, 2022 •

edited by github-actions bot

Loading

Phlair commented May 3, 2022 •

edited by github-actions bot

Loading