bigtable: fix AccessToken issues

[fanatid]

Problem

The problem is widely known that sometimes nodes lose BigTable connection (Access Token expired actually).

Previously described in different issues and pull requests, like #20336 #26217 #28728 #29692

The code itself looks completely OK, we test if the token is expired and if so set the flag to disable updates from other calls, have a timeout, and correctly handle an error, in the end we set the flag back to 'non-update status'. Everything is correct, except for one thing.... we miss that function is async and if Future would dropped during update we never finish the token update and never set the flag back to 'non-update status'. As a result, we will lose connection to BigTable. If Mutex were used instead AtomicFlag we would never had that problem because MutexGuard would be dropped anyway 😁 use AtomicFlag is ok too, but you need to be careful 😉

We currently test this patch on Triton nodes but everything looks great so far.

Summary of Changes

The obvious fix was to use Mutex instead AtomicFlag but instead I prefer to change try_refresh to 'non-async' function and use tokio::spawn for the token refresh. This is slightly more complicated but I think it's OK.

[CriesofCarrots]

Thanks for tracking this down! I made a few comments, as I think it could have been an easier review.

[codecov]

Codecov Report

Merging #34213 (c56cb61) into master (e832765) will decrease coverage by 0.1%.
Report is 7 commits behind head on master.
The diff coverage is 13.3%.

Additional details and impacted files

@@            Coverage Diff            @@
##           master   #34213     +/-   ##
=========================================
- Coverage    81.9%    81.9%   -0.1%     
=========================================
  Files         819      819             
  Lines      219350   219369     +19     
=========================================
- Hits       179698   179679     -19     
- Misses      39652    39690     +38     

[CriesofCarrots]

Thank you, much simpler! One last nit, otherwise r+ fmt and clippy being happy.

[CriesofCarrots]

Thanks!

[mergify]

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

[CriesofCarrots]

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

Thanks, mergify! This is a bug fix, and very limited in scope.