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 concurrent provider config reloads #2276

Merged

Conversation

mjeri
Copy link
Contributor

@mjeri mjeri commented Oct 18, 2017

This PR fixes problems with concurrent provider reloads in relation with the throttling functionality of the provider reloads.

The failure scenario before looked the following way:

  • given we have a ProvidersThrottleDuration of 2 seconds and two providers
    configured: Provider A and B.
  • a configuration update comes in of provider A at second 0. This config
    gets applied.
  • a configuration update comes in of provider B at second 1. The config
    reload gets delayed.
  • a configuration update comes in of provier A at second 1.5. This
    config reload gets delayed.
  • it is second 2.0 and the last config update of provider A gets
    applied, whereas the config update of provider B is lost.

This PR fixes this problem by applying the config reload throttling individually for each provider.

Note: As I removed the last usage of concurrent-map with this PR, I also removed the dependency in glide and the vendor directory.

Copy link
Contributor

@nmengin nmengin left a comment

Choose a reason for hiding this comment

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

LGTM 👏 🕙

Copy link
Contributor

@timoreimann timoreimann left a comment

Choose a reason for hiding this comment

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

LGTM.

@mjeri
Copy link
Contributor Author

mjeri commented Nov 9, 2017

Anyone else wants to have a look 👼

I would go and rebase after the final approve if this is OK.

@ldez
Copy link
Member

ldez commented Nov 9, 2017

@marco-jantke could you solve conflicts before?

@mjeri mjeri force-pushed the fix-concurrent-provider-config-reloads branch from f6f0b72 to f6784cb Compare November 9, 2017 10:50
@mjeri
Copy link
Contributor Author

mjeri commented Nov 9, 2017

Yes, sure thing. Rebased.

@ldez
Copy link
Member

ldez commented Nov 9, 2017

@marco-jantke could you rebase again?

@ldez
Copy link
Member

ldez commented Nov 14, 2017

@marco-jantke Are you here? 😄

@mjeri
Copy link
Contributor Author

mjeri commented Nov 15, 2017

Yes I am, at least kind of.. :D

I already tried to rebase, but there are some real merge conflicts now that I have to take care of. Sorry, I am very busy these weeks/months and try to come back to it soonish.

@ldez ldez added this to the 1.5 milestone Nov 16, 2017
@nmengin nmengin force-pushed the fix-concurrent-provider-config-reloads branch from f6784cb to 2b8e1c1 Compare November 16, 2017 16:40
@mjeri mjeri force-pushed the fix-concurrent-provider-config-reloads branch 2 times, most recently from 1536d93 to 2fb4084 Compare November 17, 2017 08:34
@mjeri
Copy link
Contributor Author

mjeri commented Nov 17, 2017

@ldez rebase is done.

Thanks @nmengin for the help :)

Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

Marco Jantke and others added 2 commits November 17, 2017 09:14
This commit fixes problems with concurrent provider reloads in relation
with the throttling functionality of the provider reloads.

The failure scenario before looked the following way:

Given we have a ProvidersThrottleDuration of 2 seconds and two providers
configured: Provider A and B.

- a configuration update comes in of provider A at second 0. This config
gets applied.
- a configuration update comes in of provider B at second 1. The config
  reload gets delayed.
- a configuration update comes in of provier A at second 1.5. This
  config reload gets delayed.
- it is second 2.0 and the last config update of provider A gets
  applied, whereas the config update of provider B is lost.
@traefiker traefiker force-pushed the fix-concurrent-provider-config-reloads branch from 2fb4084 to 08faafa Compare November 17, 2017 09:14
@traefiker traefiker merged commit cdab6b1 into traefik:master Nov 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants