Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BaseByteArrayJournalDao lacks resiliency against stream failure #497

Closed
Arkatufus opened this issue Dec 6, 2024 · 4 comments · Fixed by #521
Closed

BaseByteArrayJournalDao lacks resiliency against stream failure #497

Arkatufus opened this issue Dec 6, 2024 · 4 comments · Fixed by #521

Comments

@Arkatufus
Copy link
Contributor

Arkatufus commented Dec 6, 2024

BaseByteArrayJournalDao does not check to see if any of the streams it uses failed/completed/terminated, causing the journal to fail with no way of recovering, the only way to recover is to restart the node.

Chain of event:
image

image

image

@Arkatufus
Copy link
Contributor Author

Possible scenario:

  1. An exception/failure/timeout happened inside the SelectAsync() stage while a write was happening, stopping the stream (first figure).
  2. The exception did not get propagated, causing an NRE to happen when it is being propagated up the stream (second figure).
  3. The journal was not aware that the stream has stopped and failed to restart either itself or the stream, poisoning the persistence system (third figure).

@to11mtm
Copy link
Member

to11mtm commented Jan 6, 2025

This is confusing to me, primarily because AFAIK we have a fairly simple SelectAsync stage that should prevent stream stop normally:

               .SelectAsync(
                    JournalConfig.DaoConfig.Parallelism,
                    async promisesAndRows =>
                    {
                        try
                        {
                            await WriteJournalRows(promisesAndRows.Rows);
                            foreach (var taskCompletionSource in promisesAndRows.Tcs)
                                taskCompletionSource.TrySetResult(NotUsed.Instance);
                        }
                        catch (Exception e)
                        {
                            foreach (var taskCompletionSource in promisesAndRows.Tcs)
                                taskCompletionSource.TrySetException(e);
                        }

                        return NotUsed.Instance;
                    })

I'm honestly confused by this one, unless something with how shutdownToken (the only thing that could trigger a cancellation where the error makes sense in my head), and it's use in calls to ExecuteWithTransactionAsync are done is a culprit.

🤔 I take it half back. I guess the one other thing to look into, is whether linq2db has some of their own cancellationtoken stuff going on and however we are hitting it is wrong. I will note that if we give a good repo, it will get fixed (although providing our own PR may help that go faster >_<)

@Arkatufus
Copy link
Contributor Author

@to11mtm I'm actively working on this myself, the original NRE should be fixed in Akka 1.5.38, but seems like this still results in the same behavior where the stream died and the journal couldn't progress. The only way to recover from this failure would be to restart the affected nodes.

Image

@Arkatufus
Copy link
Contributor Author

So, so far, it looks like this is either a failure inside SelectAsync Task completion handling itself or still an error in exception propagation from downstream stages, still trying to create a repro unit test to figure this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants