Skip to content

Commit

Permalink
fix(bigquery): Make exponential backoff retry second based (#2212)
Browse files Browse the repository at this point in the history
* fix(bigquery): Make exponential backoff retry second based

- Currently these retries are doing retries in milliseconds, which is way too aggressive, the current retry steps are (1ms, 2ms, 4ms, 8ms, 16ms, 32ms, 64ms, 128ms, 256ms, 512ms, 1024ms, ...), it's highly likely the first 7 retries will fail with rate limiting specially in a big workload (specifically this is causing an issue in the Spark bigquery connector), I suspect the exponential retries here were meant to be in seconds (though even if it wasn't meant to, this might be a slightly better approach to reduce the load on the servers
- The new steps would be: 1s, 2s, 4s, 8s, 16s, 32s, 60s (repeated till we exhaust the 5 minutes max retries)

* Update retry base to 50ms

* Update tests

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* Avoid long overflow

- Make the comparison happen on the double side then cast to long at the end to avoid overflow happening from the calculation itself
- This was actually a bug from before, just wasn't happening because there was no 50 multiplier to get it into that state!

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
  • Loading branch information
mina-asham and gcf-owl-bot[bot] committed Nov 1, 2023
1 parent 308e9d6 commit eff4a09
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
Expand Up @@ -590,7 +590,7 @@ private void maybeWaitForInflightQuota() {

@VisibleForTesting
static long calculateSleepTimeMilli(long retryCount) {
return Math.min((long) Math.pow(2, retryCount), 60000);
return (long) Math.min(Math.pow(2, retryCount) * 50, 60000);
}

@VisibleForTesting
Expand Down
Expand Up @@ -518,8 +518,8 @@ public void testStreamNameMismatch() throws Exception {

@Test
public void testExponentialBackoff() throws Exception {
assertThat(ConnectionWorker.calculateSleepTimeMilli(0)).isEqualTo(1);
assertThat(ConnectionWorker.calculateSleepTimeMilli(5)).isEqualTo(32);
assertThat(ConnectionWorker.calculateSleepTimeMilli(0)).isEqualTo(50);
assertThat(ConnectionWorker.calculateSleepTimeMilli(5)).isEqualTo(1600);
assertThat(ConnectionWorker.calculateSleepTimeMilli(100)).isEqualTo(60000);
}

Expand Down

0 comments on commit eff4a09

Please sign in to comment.