adding context timeouts for management queries based on reload interval#390
adding context timeouts for management queries based on reload interval#390rasamala83 wants to merge 7 commits intopaypal:mainfrom
Conversation
| return | ||
| case <-timeTicker.C: | ||
| //Periodic data loading | ||
| racMaint(&ctx, shard, db, racSQL, cmdLineModuleName, prev, waitTime/2) |
There was a problem hiding this comment.
Let us use a configurable query timeout for maintenance queries. Here, the wait time could vary across pools.
| //First time data loading | ||
| racMaint(&ctx, shard, db, racSQL, cmdLineModuleName, prev, waitTime/2) | ||
|
|
||
| timeTicker := time.NewTicker(waitTime) |
There was a problem hiding this comment.
defer timeTicker.Stop() to release resources.
There was a problem hiding this comment.
Fixed timeTicker.Stop() and reset
| case <-ctx.Done(): | ||
| logger.GetLogger().Log(logger.Alert, "Application main context has been closed, so exiting from racmaint data reload.") | ||
| return | ||
| case <-timeTicker.C: |
There was a problem hiding this comment.
I'm fine with this but why do we need to introduce the timeTicker? I feel the existing code is simpler.
| db, err = openDb(shard) | ||
| if err == nil { | ||
| err = loadMap(ctx, db) | ||
| err = loadMap(&ctx, db, reloadInterval/2) |
There was a problem hiding this comment.
Same as above. Let us use a configurable query timeout for maintenance queries
| loadWhitelist(&ctx, db, reloadInterval/2) | ||
| } | ||
| go func() { | ||
| reloadTimer := time.NewTimer(reloadInterval) //Periodic reload timer |
There was a problem hiding this comment.
This needs to be replaced with NewTicker as the timer fires just once.
There was a problem hiding this comment.
added reset and clean up options for timer
| if err == nil { | ||
| err = loadMap(ctx, db) | ||
| select { | ||
| case <-ctx.Done(): |
There was a problem hiding this comment.
Let us also add timeout-related tests for validation.
|
@rasamala83 Converting the PR to draft as it is work in progress. |
venkatsridhar95
force-pushed
the
main
branch
2 times, most recently
from
cd1c2cb to
7fd4baa
Compare
3 participants
No description provided.