-
Notifications
You must be signed in to change notification settings - Fork 15
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
BCN-458 handle specific decoding error when schema registry unavailable #133
BCN-458 handle specific decoding error when schema registry unavailable #133
Conversation
d8d85ca
to
267a7ba
Compare
267a7ba
to
6ecad9f
Compare
Tested on a cluster by deliberately killing the schema registry:
|
f566360
to
cd4a426
Compare
Of course after exhausing all retries.
cd4a426
to
c82aacb
Compare
2a3c667
to
4f1eb1e
Compare
Another potential issue I would like to highlight, in case it was not discussed, is that if we go this way and return |
In that case the infinite loop is something we want, isn't it? |
If we are taking about schema registry being unavailable then - yes. But I am afraid if there can be other errors, I don't know for example bad format or whatever, that we might what to skip, it came to my mind when I saw this test case? |
In those cases we won't return an |
500 type error include database errors (which should be temporary) |
Return a specific error that can be used to detect when the schema registry is unavailable. This means that, for example, number one can retry indefinitely in case such an error is encountered so as not to skip messages when consuming but otherwise respect a limited retry mechanism that might be in place.
Instead of trying to check all the net.Error, url.Error or syscall.ECONNREFUSED, the logic has simply been reduced to "in case there is an error during performing the request and we have no more retries left then the registry is not available". This means that, even if we have consecutive timeouts that exceed the retry limit for operations will be considered as the schema registry is not available.