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

fix/ issue #164: Race condition in init() of connector classes #197

Merged
merged 11 commits into from
Oct 16, 2023

Conversation

OjusWiZard
Copy link
Contributor

@OjusWiZard OjusWiZard commented Sep 3, 2023

Before submitting this PR, please make sure:

  • Your code builds clean without any errors or warnings
  • You are using the approved title ("feat/", "fix/", "docs/", "refactor/")

A description of the changes proposed in the pull request:
This PR attempts to fix the issue #164. I observed that the cause of the errors in this issue is the following sequence:

  1. The first /amm/price request is made.
  2. This calls the init() function for the first time and the promise is returned which is awaited.
  3. More subsequent /amm/price calls are made.
  4. The ready() function still returns false because the first init() call is not finished yet.
  5. So, all the subsequent /amm/price request again calls the init() function.
  6. These latter calls skipped the initialization because this._initializing was true. Hence, these init() calls finished before the first init() call.
  7. So, the latter price queries proceed as usual and fail at the getTokenBySymbol() because the tokens are yet to load from the first init() call.

Solution: The latter init() calls are made to wait while some other init() call is in progress. This is done by having a promise (this._initialized), that is unresolved whenever the init() is in progress.

Tests performed by the developer:
I tested the cases described in the issue thread around ten times. The errors didn't occur in any case.

Copy link
Contributor

@nikspz nikspz left a comment

Choose a reason for hiding this comment

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

Test performed:

  • Cloned and installed fix branch PR197 + latest development
  • Connected uniswap arbitrum and created amm_arb using it
  • started strategy, review bot showed profitability rate
  • stopped gateway, restarted Client and gateway and review strategy logs showing no errors
  • Start the strategy again and review issue was reproduced when gateway stopped and turn OFF
  • start gateway again with yarn start —passphrase=password. Gateway turned into ONLINE again and recovered
  • connected dexalot/pancakeswap and checked balance successfully after the restart
  • created/started pancakeswap amm_arb successfully
  • amm_arb order execution using uniswap arbitrum one after gateway restart: ok
  • manually built docker image successfully
  • Reviewed HB gateway compose was able to recover after gateway disconnected

@OjusWiZard OjusWiZard changed the title fix/ issue #164 fix/ issue #164: Race condition in init() of connector classes Sep 14, 2023
Copy link
Contributor

@fengtality fengtality left a comment

Choose a reason for hiding this comment

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

LGTM

@nikspz nikspz merged commit 0ff07f7 into hummingbot:development Oct 16, 2023
3 checks passed
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.

Gateway - getting price query failed error after restarting gateway connection
4 participants