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

routing: remove route cache #2950

Merged
merged 1 commit into from Apr 18, 2019

Conversation

Projects
None yet
4 participants
@joostjager
Copy link
Collaborator

commented Apr 13, 2019

This commit removes the QueryRoutes route cache. It is causing wrong
routes to be returned because not all of the request parameters are
stored.

The cache allowed high frequency QueryRoutes calls to the same
destination and with the same amount to be returned fast. This behaviour
can also be achieved by caching the request on the client side. In case
a route is invalidated because of for example a channel update,
the subsequent SendToRoute call will fail. This is a trigger to call
QueryRoutes again for a fresh route.

routing: remove route cache
This commit removes the QueryRoutes route cache. It is causing wrong
routes to be returned because not all of the request parameters are
stored.

The cache allowed high frequency QueryRoutes calls to the same
destination and with the same amount to be returned fast. This behaviour
can also be achieved by caching the request on the client side. In case
a route is invalidated because of for example a channel update,
the subsequent SendToRoute call will fail. This is a trigger to call
QueryRoutes again for a fresh route.
@halseth
Copy link
Collaborator

left a comment

Beautiful , stamp of approval 💯

@Roasbeef Roasbeef added this to the 0.6.1 milestone Apr 16, 2019

@cfromknecht
Copy link
Collaborator

left a comment

LGTM 🔥

@Roasbeef Roasbeef merged commit 373927f into lightningnetwork:master Apr 18, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on remove-route-cache at 59.776%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.