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

Open
Arkatufus opened this issue Dec 6, 2024 · 2 comments
Open

BaseByteArrayJournalDao lacks resiliency against stream failure #497

Arkatufus opened this issue Dec 6, 2024 · 2 comments

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 >_<)

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

No branches or pull requests

2 participants