joshfree
commented
1 year ago Hi @aleksanderKopec thank you for posting this detailed issue. @mssfang will follow up with you shortly!
Open aleksanderKopec opened 1 year ago
joshfree
commented
1 year ago Hi @aleksanderKopec thank you for posting this detailed issue. @mssfang will follow up with you shortly!
mssfang
commented
1 year ago @mrm9084 Can you have a look at this? It is from spring-cloud-azure-appconfiguration-config AppConfigurationRefresh
aleksanderKopec
commented
1 year ago Hello, any updates on this? It's not super high priority for now, but I would like to know if I should find some workaround.
mrm9084
commented
1 year ago @aleksanderKopec, I have been unable to replicate the issue. I'm still trying too. If you had a sample that did it, it would help identify the issue.
mrm9084
commented
1 year ago Ok. Was able to replicate this. A few things.
refreshConfigurations().block() method it fixes the issue.This is caused by the background process still running when you return already. This could work around for now, will look to see if there is a better long term fix.
aleksanderKopec
commented
1 year ago Thanks for the response.
I've tried multiple ways of adding .block() to refreshConfigurations, including running this on a Schedulers.boundedElastic() scheduler, none of them seem to work. So for example code like that doesn't work:
@GetMapping("/flag1/{flag}")
public Mono<Boolean> getFlagByName1(@PathVariable String flag, ServerWebExchange exchange) {
appConfigurationRefresh.refreshConfigurations().block();
return featureManager.isEnabledAsync(flag);
}Some other things which I tried included running something akin to this:
@GetMapping("/flag1/{flag}")
public Mono<Boolean> getFlagByName1(@PathVariable String flag, ServerWebExchange exchange) {
return Mono.just(Objects.requireNonNull(appConfigurationRefresh.refreshConfigurations().block()))
.flatMap((ignored) -> featureManager.isEnabledAsync(flag))
.subscribeOn(Schedulers.boundedElastic());
}or this
@GetMapping("/flag1/{flag}")
public Mono<Boolean> getFlagByName1(@PathVariable String flag, ServerWebExchange exchange) {
return appConfigurationRefresh.refreshConfigurations()
.flatMap(ignored -> featureManager.isEnabledAsync(flag))
.subscribeOn(Schedulers.boundedElastic());
}None of the code listed before worked. I've managed to make it work though, with this code:
@GetMapping("/flag1/{flag}")
public Mono<Boolean> getFlagByName1(@PathVariable String flag, ServerWebExchange exchange) {
return Mono.just(1)
.flatMap(ignored -> appConfigurationRefresh.refreshConfigurations())
.flatMap(ignored -> featureManager.isEnabledAsync(flag))
.subscribeOn(Schedulers.boundedElastic());
}I have no idea why this works, and the one in third code block doesn't, from my understanding they should be the same. But it seems the issue is that in other configurations the actual refresh is run on the default parallel thread pool, rather than the specified boundedElastic, and you can't run .block() on parallel thread pool.
So yeah, the workaround works for our use-case, does exactly what I expect it to do, so I guess the issue could be closed if you prefer to. But I'm pretty sure it's still a valid bug, especially the third code block seems like something that should work.
mrm9084
commented
1 year ago @aleksanderKopec, I agree it is a bug. I was able to replicate the error, and just any block fixed it for me. What annotation are you using on the class, it may effect it.
aleksanderKopec
commented
1 year ago These are all annotations I use on the Controller
@RestController
@RequestMapping("/v1/feature-flags")
@RequiredArgsConstructor
Describe the bug When attempting to refresh the configuration values using AppConfigurationRefresh, we are getting an exception because of an .block() in the configuration refresh code.
Exception or Stack Trace
To Reproduce Steps to reproduce the behavior: Run
AppConfigurationRefresh.refreshConfigurations()when the refresh interval has passed (so the actual refresh is run).Code Snippet
Expected behavior The configuration values should refreshed and no exception should be thrown.
Setup (please complete the following information):
Additional context We are getting some relevant warnings before the exception is thrown
Information Checklist Kindly make sure that you have added all the following information above and checkoff the required fields otherwise we will treat the issuer as an incomplete report