archaius
archaius copied to clipboard
FixedPollingStrategy blocks if remote source is unavailable
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 ?
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.
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?
https://github.com/Netflix/archaius/compare/2.x...siddheshshirsat:Issue548
Nice catch! :+1: I wish I looked here before production incident...
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 .
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.