Relay timeouts #918
Relay timeouts #918
Conversation
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
| "sync" | ||
| ) | ||
|
|
||
| // CachedAccessor is an interface for accessing a resource that is cached. It assumes that cache misses | ||
| // are expensive, and prevents multiple concurrent cache misses for the same key. | ||
| type CachedAccessor[K comparable, V any] interface { | ||
| // Get returns the value for the given key. If the value is not in the cache, it will be fetched using the Accessor. | ||
| Get(key K) (V, error) | ||
| // If the context is cancelled, the function may abort early. If multiple goroutines request the same key, | ||
| // cancellation of one request will not affect the others. |
There was a problem hiding this comment.
nit: the cancellation of the first request can affect others (all of them get error)
There was a problem hiding this comment.
The way I implemented this class, cancellation of the first request should not cause there to be an error for the following requests. Do you see a bug in the implementation?
There was a problem hiding this comment.
Ok, I think it's about how to read "not affect", the failure of fetch is not the failure of first request, then this comment is correct. Anyway, it's fine to keep this.
relay/cache/cached_accessor.go
Outdated
| } | ||
| result.wg.Add(1) | ||
| result.sem.TryAcquire(1) |
There was a problem hiding this comment.
This always succeeds, should it just Acquire()?
There was a problem hiding this comment.
This will always succeed either way, since this happens right after the semaphore is created and since no other goroutine has access to the semaphore yet. The reason why I use TryAcquire() is that it doesn't return an error. If I use Acquire() then I have to add error handling, even though it's impossible for an error to be returned.
With this in mind, are you ok with TryAcquire(1) here?
There was a problem hiding this comment.
Does _ = result.sem.Acquire() work (with comment that this should always succeed)?
TryAquire() returns a boolean, so it's the same in the sense that the return value is ignored.
So both will work, the difference I think is that TryAcquire reads like it doesn't care about whether it has grabbed the resource.
There was a problem hiding this comment.
I've switched this to _ = result.sem.Acquire(context.Background(), 1)
Signed-off-by: Cody Littley <[email protected]>
Signed-off-by: Cody Littley <[email protected]>
Why are these changes needed?
Adds proper timeouts to relay operations.
Checks