archaius icon indicating copy to clipboard operation
archaius copied to clipboard

FixedPollingStrategy blocks if remote source is unavailable

Open fcappi opened this issue 6 years ago • 5 comments

Hi, Currently, when you try to instantiate a PollingDynamicConfig, using the FixedPollingStrategy, the execution stucks on the following while(true) condition if the remote source is unavailable:

https://github.com/Netflix/archaius/blob/3a56eb807cf1465212e5d65603a54b24dbeea8a4/archaius2-core/src/main/java/com/netflix/archaius/config/polling/FixedPollingStrategy.java#L44-L72

The problem is that if you are using Guice to provide the property source, it will block Guice from resolving this dependency.

My suggestion is to remove the while(true) and let the scheduler handle the attempts to read from the remote source.

What do you guys think ?

fcappi avatar May 05 '18 14:05 fcappi

I don't think the issue is specific to Guice or a remote service. Any Exception thrown from the reader's call() method will result in an indefinite loop.

https://github.com/Netflix/archaius/blob/3a56eb807cf1465212e5d65603a54b24dbeea8a4/archaius2-core/src/main/java/com/netflix/archaius/config/PollingDynamicConfig.java#L82

I agree that even the first attempt to perform the update should be done with the ScheduledExecutorService.

siddheshshirsat avatar Dec 24 '19 02:12 siddheshshirsat

The first attempt to perform the update if done with the ScheduledExecutorService results in race conditions in reading values immediately after creating the config instance.

One solution could be to just get rid of the infinite loop block and do a best-attempt reading for the first time and then defer the rest of the calls to the reader to the ScheduledExecutorService.

I have the fix ready for this in my local workspace but don't have permissions to raise a CR or push to a remote branch. Can someone help with that?

siddheshshirsat avatar Dec 24 '19 20:12 siddheshshirsat

https://github.com/Netflix/archaius/compare/2.x...siddheshshirsat:Issue548

siddheshshirsat avatar Dec 27 '19 18:12 siddheshshirsat

Nice catch! :+1: I wish I looked here before production incident...

BenderRodrigez avatar Jul 25 '20 20:07 BenderRodrigez

Thank you.

On Sat, Jul 25, 2020, 13:08 Равиль Гиниятуллин [email protected] wrote:

Nice catch! 👍 I wish I looked here before production incident...

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Netflix/archaius/issues/548#issuecomment-663900798, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACRG4BFZURSR3DY4DW4XPMDR5M3U3ANCNFSM4E6OUFZA .

siddheshshirsat avatar Jul 25 '20 20:07 siddheshshirsat

Won't fix.

Although this issue was reported against v1, v2 has equivalent behavior and this is a design choice. We believe that allowing an application to come up without being able to receive remote config overrides has the potential of causing even worse issues, because any config that is only read at boot time would be silently set to the "non-overridden" value, causing hard-to-debug problems from an inconsistent fleet.

rgallardo-netflix avatar Feb 15 '23 22:02 rgallardo-netflix