Update TTL of transactions upon finalisation to 500 ms #235
Conversation
|
Your Render PR Server URL is https://chproxy-pr-235.onrender.com. Follow its progress at https://dashboard.render.com/static/srv-cclc1hta4990ukkqu4vg. |
gontarzpawel
force-pushed
the
feat/ttl-update-transactions
branch
from
5abf9b0 to
35e0238
Compare
gontarzpawel
changed the title
| @@ -21,6 +22,9 @@ type TransactionRegistry interface { | |||
| Status(key *Key) (TransactionStatus, error) | |||
| } | |||
|
|
|||
| // transactionEndedTTL amount of time transaction record is kept after being updated | |||
| const transactionEndedTTL = 500 * time.Millisecond | |||
There was a problem hiding this comment.
IMHO it should be configurable because some people will tell you they want 0 msec
There was a problem hiding this comment.
I was thinking about it too, but it makes few sense to expose it in the configuration because it's a technical detail of the implementation. I'd say instead of exposing it, we could expose a toggle feature flag to disable thundering herd explicitly
There was a problem hiding this comment.
here is my proposal: let's ship this feature first and let's see how people respond.
| } | ||
|
|
||
| // move ttls of mini redis to trigger the clean up transaction | ||
| s.FastForward(updatedTTL) |
There was a problem hiding this comment.
nice trick, I was not aware of this function
| @@ -79,3 +79,51 @@ func TestFailRedisTransaction(t *testing.T) { | |||
| t.Fatalf("unexpected: transaction should curry fail reason") | |||
| } | |||
| } | |||
|
|
|||
| func TestCleanupRedisTransaction(t *testing.T) { | |||
There was a problem hiding this comment.
IMHO, you should do 2 tests: one on failure (the one you did) and one on completion.
gontarzpawel
force-pushed
the
feat/ttl-update-transactions
branch
from
b0547c9 to
930462c
Compare
Description
Fix for the issue #230
500 msupon finalisationPull request type
Please check the type of change your PR introduces:
Checklist
Does this introduce a breaking change?
Further comments
I took a liberty also to do a small refactor in proxy.go. I moved logic away from
serveFromCachethat decides if query can be served from cache.