Conversation
Signed-off-by: machichima <nary12321@gmail.com>
Signed-off-by: machichima <nary12321@gmail.com>
Signed-off-by: machichima <nary12321@gmail.com>
Signed-off-by: machichima <nary12321@gmail.com>
Signed-off-by: machichima <nary12321@gmail.com>
Signed-off-by: machichima <nary12321@gmail.com>
| go s.repo.ActionRepo().WatchActionUpdates(ctx, actionID.Run, updatesCh, errsCh) | ||
|
|
||
| lastPhase := action.Phase | ||
| const maxEvents = 500 |
There was a problem hiding this comment.
Just set a reasonable number here first, normally we will not get to this vaue. We can decide whether we want to increase/decrease it later one
Signed-off-by: machichima <nary12321@gmail.com>
| logger.Infof(ctx, "Received WatchClusterEvents request for: %s/%s", actionID.Run.Name, actionID.Name) | ||
|
|
||
| // Step 1: Send existing events from current DB state | ||
| action, err := s.repo.ActionRepo().GetAction(ctx, actionID) |
There was a problem hiding this comment.
Should we watch first then query action db? I t is likely that some updates might be lost between the time window
runs/service/run_service.go
Outdated
| } | ||
| // Filter to this specific action | ||
| if updated.Name != actionID.Name { | ||
| continue |
There was a problem hiding this comment.
Looks like this continue here will makes the program go back to the top while loop and do the drain process again. Is there any reason for doing so?
There was a problem hiding this comment.
Update: This is to prevent the event miss. I tried to skip when the update is not from target action, we will only get partial event (see following).
I think it's a bug now and my implementation is a workaround. Will try to deal with this in follow-ups
Signed-off-by: machichima <nary12321@gmail.com>
Signed-off-by: machichima <nary12321@gmail.com>
Signed-off-by: machichima <nary12321@gmail.com>
| // Revalidate this bucket is still current before writing. | ||
| w.mu.RLock() | ||
| stillCurrent := w.objectCache[objectKey] == eventInfos | ||
| w.mu.RUnlock() | ||
| if !stillCurrent { | ||
| // eventInfos being deleted/changed, we should get the newest object again | ||
| eventInfos.mu.Unlock() | ||
| continue |
There was a problem hiding this comment.
do we still need revalidate if we already have a lock per event?
There was a problem hiding this comment.
Fix by not deleting eventInfo when it's empty in 7976995
Also use sync.Map instead for objectCache
Signed-off-by: machichima <nary12321@gmail.com>
Signed-off-by: machichima <nary12321@gmail.com>
Signed-off-by: machichima <nary12321@gmail.com>
Signed-off-by: machichima <nary12321@gmail.com>
Signed-off-by: machichima <nary12321@gmail.com>
Signed-off-by: machichima <nary12321@gmail.com>
Signed-off-by: machichima <nary12321@gmail.com>
Signed-off-by: machichima <nary12321@gmail.com>
3 participants
Tracking issue
Why are the changes needed?
Previously
WatchClusterEventsincorrectly return action phase rather than cluster events.What changes were proposed in this pull request?
AdditionalReasonsin phase infoAdditionalReasonsand put intoClusterEventsin action eventWatchClusterEventget fromClusterEventsHow was this patch tested?
Labels
Please add one or more of the following labels to categorize your PR:
This is important to improve the readability of release notes.
Setup process
Screenshots
Check all the applicable boxes
Related PRs
Docs link
main