Support several RPC URLs #59

fvictorio · 2018-08-02T11:06:18Z

Closes #28.

This PR allows configuring several URLs in the HOME_RPC_URL and FOREIGN_RPC_URL environment variables.

To do this, two things were necessary: creating a new web3 provider that accepts several URLs (see HttpListProvider) and creating a RpcUrlsManager service that, given a function, will try it with each available URL until one of them works. This service also centralizes the access to the URLs, instead of having process.env.HOME_RPC_URLs all over the code.

To test this, I created a simple express app that acts as a proxy and logs each request. Then I started two proxy apps that redirect everything to the port 8545 and two other that redirect to the port 8546, where I had two parity nodes running. Then I configured the bridge to use two of the proxies as home URLs and the other two as foreign URLs. Finally, I killed and restarted the proxies in several combinations while sending transactions to check that everything was working.

The code for this express proxy is:

const proxy = require('express-http-proxy')
const app = require('express')()

const { PORT_FROM, PORT_TO } = process.env

app.use('/', proxy(() => {
  console.log('Request')
  return `http://localhost:${PORT_TO}`
}))

app.listen(PORT_FROM)

Then, in four different terminals, I did:

PORT_FROM=8555 PORT_TO=8545 node index.js
PORT_FROM=8565 PORT_TO=8545 node index.js
PORT_FROM=8556 PORT_TO=8546 node index.js
PORT_FROM=8566 PORT_TO=8546 node index.js

And in the .env file I used:

HOME_RPC_URL=http://localhost:8555,http://localhost:8565
FOREIGN_RPC_URL=http://localhost:8556,http://localhost:8566

Add a RpcUrlsManager that, given the (comma separated) lists of home and foreign RPC URLs, parses them and exposes them. This does not change the behavior of the system, since only the first URL is used.

This way is more explicit that only one of several possible URLs is being used. Besides, ideally the full list of URLs should be used everywhere.

fvictorio · 2018-08-02T11:09:52Z

Two more things:

First, there's something pending. Right now, if all URLs fail, the request (or web3 method) will fail. But the original issue says that it should wait and retry after a while. This is easy to add using retry-promise, but first let's check that everything is fine here, since this PR is hard enough to review as it is.

Second, I think the provider could be useful in other projects, so maybe it could be a separate project published in NPM and we could use it from there. Are you OK with that?

akolotov

I think the provider could be useful in other projects, so maybe it could be a separate project published in NPM and we could use it from there. Are you OK with that?

Currently I see that the functionality is too tied with 'home' and 'foreign' items. What exactly are you suggesting to move to a separate project?

akolotov

Is my understanding correct and you in are trying to start every time with the first URL in the list even if was found that it does not response? Will it impact performance if we are trying to handle hundreds of transactions?
Does it make sense to remember last successful URL in order to start from it rather than the first element of the URL list?

fvictorio · 2018-08-02T18:44:46Z

Currently I see that the functionality is too tied with 'home' and 'foreign' items. What exactly are you suggesting to move to a separate project?

Sorry, I was referring to the web3 provider. I think it's not tied to anything, so it should be usable in any project that could benefit from fallback URLs. I actually already moved it to its own repository, mainly for testing it, since doing that requires installing some very specific devDependencies. If you are OK with this idea, I can publish it in npm and update this PR to use it.

Is my understanding correct and you in are trying to start every time with the first URL in the list even if was found that it does not response? Will it impact performance if we are trying to handle hundreds of transactions?
Does it make sense to remember last successful URL in order to start from it rather than the first element of the URL list?

No, I think both the RpcUrlManager and the provider are using the last URL that worked. Let me know if you think that's not working.

akolotov · 2018-08-02T19:22:17Z

I actually already moved it to its own repository.

If it is an open repo, could you send a link?

fvictorio · 2018-08-03T12:33:31Z

Sure, I just created the repo here and is published in npm. There are several things that could be done to improve it (support more node versions, support browser usage), but it can be used here as it is.

…over-urls

akolotov · 2018-08-10T16:54:13Z

@rstormsf please take a look at this PR. Thanks!

vbaranov

@fvictorio
There are some minor comments for future improvements:

first of all, npm package http-list-provider is not included to PR yet. HttpListProvider is defined inside current repo
README.md with an example of usage is missing in suggested npm pakage
Description of how to test the utility is missing
It is better to add launch and stop of ganache-cli to test script (or at least mention, that ganache-cli should be launched before a test)

vbaranov · 2018-08-13T15:13:45Z

src/utils/HttpListProvider.js

+  const currentIndex = this.currentIndex // eslint-disable-line prefer-destructuring
+
+  if (retries === this.urls.length) {
+    callback(new Error('Request failed for all urls'))


@fvictorio @akolotov what if for some reasons all of the RPC endpoints are not accessible in a short-term period, in this case, send method will fail here. What if to increase the level of service availability? I mean, to try to reach all RPC endpoints again. Introduce retriesOuter is equal, let's, say 3 - how many times send will try to reach all RPC endpoints. If all those tries will fail, then the method will fail. What do you think?

Thanks @vbaranov for the comment! Actually it was requested in the original issue (#28) and @fvictorio said (#59 (comment)) that is will added later after the review of main part of changes.

oh, I see. Nevertheless, the rest looks good to me

fvictorio · 2018-08-13T17:09:56Z

@vbaranov Thanks for the comments. I added a commit to use the provider as a dependency. I also added a README to the repo.

Regarding the retry, I think we should create a separate issue and do it in another PR. The worst case scenario (all URLs fail) is not worse than what we have now (the single URL fails), so I think it's safe to merge this.

akolotov

So, let's merge this PR.
And please open a new issue to introduce retry for the list of URLs.

fvictorio · 2018-08-14T13:22:57Z

@vbaranov I'm going to merge this, but any other comments regarding this are welcome.

fvictorio added 9 commits July 31, 2018 10:28

Add tryEach function

2c3a2e6

Centralize the access to RPC URLs

0ad9908

Add a RpcUrlsManager that, given the (comma separated) lists of home and foreign RPC URLs, parses them and exposes them. This does not change the behavior of the system, since only the first URL is used.

Use multiple RPC URLs in both web3 and in raw requests

7c98abb

Add log when all blocks are already processed

1339b84

Use fallback URLs in scripts

d33880d

Remove getHomeUrl and getForeignUrl functions

6b0180a

This way is more explicit that only one of several possible URLs is being used. Besides, ideally the full list of URLs should be used everywhere.

Avoid race condition in HttpListProvider

dbca152

Add tryEach method to RpcUrlsManager and use it in sendRawTx

f7645e2

Remove tryEachHelper and clone urls with lodash

6c7df7f

fvictorio requested review from patitonar, rstormsf and akolotov August 2, 2018 11:06

ghost assigned fvictorio Aug 2, 2018

ghost added the review label Aug 2, 2018

akolotov reviewed Aug 2, 2018

View reviewed changes

Add env variables to Travis so that tests pass

34a2f2f

patitonar approved these changes Aug 6, 2018

View reviewed changes

fvictorio added 2 commits August 6, 2018 14:37

Put all env variables in the same item

7d61334

Merge branch 'develop' into #28-ability-to-configure-several-rpc-fail…

ee9fb3b

…over-urls

vbaranov reviewed Aug 13, 2018

View reviewed changes

Use http-list-provider as dependency

caa37a0

akolotov approved these changes Aug 13, 2018

View reviewed changes

fvictorio merged commit c24917a into develop Aug 14, 2018

fvictorio deleted the #28-ability-to-configure-several-rpc-failover-urls branch August 14, 2018 13:23

ghost removed the review label Aug 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support several RPC URLs #59

Support several RPC URLs #59

fvictorio commented Aug 2, 2018

fvictorio commented Aug 2, 2018

akolotov left a comment

akolotov left a comment

fvictorio commented Aug 2, 2018 •

edited

Loading

akolotov commented Aug 2, 2018 •

edited

Loading

fvictorio commented Aug 3, 2018

akolotov commented Aug 10, 2018

vbaranov left a comment

vbaranov Aug 13, 2018

akolotov Aug 13, 2018

vbaranov Aug 13, 2018

fvictorio commented Aug 13, 2018

akolotov left a comment

fvictorio commented Aug 14, 2018

Support several RPC URLs #59

Support several RPC URLs #59

Conversation

fvictorio commented Aug 2, 2018

fvictorio commented Aug 2, 2018

akolotov left a comment

Choose a reason for hiding this comment

akolotov left a comment

Choose a reason for hiding this comment

fvictorio commented Aug 2, 2018 • edited Loading

akolotov commented Aug 2, 2018 • edited Loading

fvictorio commented Aug 3, 2018

akolotov commented Aug 10, 2018

vbaranov left a comment

Choose a reason for hiding this comment

vbaranov Aug 13, 2018

Choose a reason for hiding this comment

akolotov Aug 13, 2018

Choose a reason for hiding this comment

vbaranov Aug 13, 2018

Choose a reason for hiding this comment

fvictorio commented Aug 13, 2018

akolotov left a comment

Choose a reason for hiding this comment

fvictorio commented Aug 14, 2018

fvictorio commented Aug 2, 2018 •

edited

Loading

akolotov commented Aug 2, 2018 •

edited

Loading