Fix: Add error status handling for ingestion and sync source #283
Conversation
| // Fetch ingestion error from run.status in go routine so that we don't block | ||
| runName := ws.Status.IngestionRunName | ||
| go func() { | ||
| for { | ||
| stop, err := updateIngestionError(req, ws, runName) | ||
| if err != nil { | ||
| logger.Errorf("failed to update ingestion error: %s", err) | ||
| break | ||
| } | ||
|
|
||
| if stop { | ||
| break | ||
| } | ||
| time.Sleep(time.Second * 5) | ||
| } | ||
| }() |
There was a problem hiding this comment.
The above call to compileFileStatuses will only return once the run is complete (either success or failure). This would mean that this coroutine would only run once.
Can it be removed?
There was a problem hiding this comment.
The problem here is that even when run is finished, the run.status.error is not propagated from other controller(run controller) here so it needs to wait for a bit for the error to be propagated.
There was a problem hiding this comment.
If that's the case, then I think this should be a controller handler.
There was a problem hiding this comment.
we are wiping out ws.Status.IngestionRunName in the end, so other controller won't have information about the latest run. So it might be easy to just add logic to to retrieve the error.
There was a problem hiding this comment.
I pushed a commit with how I would understand this to work if we didn't use a goroutine. Let me know what you think about it. If it doesn't work like you expect, then we can just drop the commit I added.
StrongMonkey
force-pushed
the
fix-more-bugs
branch
from
e0bb39e to
85fd6b4
Compare
StrongMonkey
force-pushed
the
fix-more-bugs
branch
from
ee14f01 to
a42f686
Compare
Signed-off-by: Daishan Peng <[email protected]>
Signed-off-by: Daishan Peng <[email protected]>
Signed-off-by: Daishan Peng <[email protected]>
Signed-off-by: Donnie Adams <[email protected]>
Signed-off-by: Daishan Peng <[email protected]>
StrongMonkey
force-pushed
the
fix-more-bugs
branch
from
3882246 to
c5a9016
Compare
| @@ -249,63 +249,44 @@ func (a *Handler) UpdateFileStatus(req router.Request, _ router.Response) error | |||
| return err | |||
| } | |||
|
|
|||
| if run.Status.State.IsTerminal() { | |||
There was a problem hiding this comment.
@thedadams There is a chance that when run is not in terminal state and we are not immediately setting ws.Status.IngestionRunName = '', the controller will reprocess the logs from the run events, which could be a lot of computes.
So I think the best way to address this without a goroutine is to add a new field lastIngestionRunName to keep track it the run and update error based on that.
This PR addes the following things
#186