I have a following code that is obtaining a session token by placing a http call to the 3rd party service. Every now and then I get 500 or 503 when the 3rd party service is flooded with requests.
I am here implementing super primitive back-off policy. Ideally, I want here a rate limiter, but I am going with a simple quick solution for now.
The thing is that this class is a Spring Component (@Component) and is used in a threaded environment, multiple threads would be using this SessionHandler class.
public String obtainSessionToken(String orgId,
String userId) throws UiSessionTokenGenerationException {
String sessionTokenJson;
int retriesRemaining = maxRetries;
while (true) {
try {
sessionTokenJson = browserSessionTokenService.getBrowserSessionToken(
orgId,
userId);
break;
} catch (Exception e) {
--retriesRemaining;
String message = "Error generating UI session token" ;
LOG.warn("{}, retriesRemaining={}", message, retriesRemaining);
if(retriesRemaining == 0)
throw new UiSessionTokenGenerationException(message);
int sleepMultiplier = maxRetries - retriesRemaining;
sleep(sleepMultiplier);
}
}
return extractSessionFromResponse(c2cTenant, recordId, sessionTokenJson);
}
private void sleep(int sleepMultiplier) {
try {
Thread.sleep(minSleepPeriod * sleepMultiplier);
} catch (Exception e) {
throw new RuntimeException(e);
}
}
I wonder if it will be a bottleneck granted that all the objects in Spring are created by default in the singleton scope. It shouldn't be, right?
===========UPDATE==========
Let me clarify my question: I am assuming each calling thread will have its own execution and does not interfere with another thread's execution unless there is some code in the singleton scoped class which is shared by all calling threads.Since thread sleeps are applied to each calling thread, the overall throughput will not suffer. Is it a valid assumption? And, as such this class can remain a component (singleton).
No, just because a the bean is a singleton does not mean that all threads have to wait for any other thread to complete first. You need to write code that makes sections of code to be executed mutually exclusive by different threads. One such mechanism is the synchronized keyword and you do not have that in your code.
Singletons can become problematic if they have state, because the state would be shared across all consumers (because there is only a single instance of the singleton).
int retriesRemaining = maxRetries; // local variable
while (true) {
try {
// ...
} catch (Exception e) {
--retriesRemaining;
// ...
if(retriesRemaining == 0)
throw new UiSessionTokenGenerationException(message);
int sleepMultiplier = maxRetries - retriesRemaining; // local variable
sleep(sleepMultiplier);
}
}
All state that your function uses is local to the function itself and not shared with any other function or the singleton instance. Both retriesRemaining and sleepMultiplier are local variables (of primitive type: both are int), so it is impossible that they will affect other threads.
Finally, Thread#sleep only affects the current thread, all other threads are unaffected (unless they are waiting for the sleeping thread to leave a critical section, e.g. a synchronized method or block).
As for the throughput of your application: perform experiments and do measurements, that's the only way to prove your hypothesis for your specific application.