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

MDB throws a MDB_NOTFOUND exception when the database directory already exists #5424

Merged
merged 7 commits into from
Dec 13, 2021

Conversation

Arkatufus
Copy link
Contributor

@Arkatufus Arkatufus commented Dec 10, 2021

Closes #5423

  • Create a bug reproduction spec
  • Create fix

@Arkatufus Arkatufus self-assigned this Dec 10, 2021
@Arkatufus Arkatufus marked this pull request as ready for review December 10, 2021 23:19
{
try
using (var env = GetLightningEnvironment())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might as well re-arrange the try...catch block to include the LMDB environment initialization code so it can catch exceptions thrown there

{
try
{
foreach (var entry in _pending)
using (var db = tx.OpenDatabase(DatabaseName))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, rearranged the try...catch block

Copy link
Member

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Aaronontheweb
Copy link
Member

@Arkatufus

Akka.DistributedData.Tests.LightningDb.LmdbDurableStoreSpec.Lmdb_should_not_throw_when_opening_existing_directory

Failed on Windows with .NET Framework.

@Arkatufus
Copy link
Contributor Author

I can't replicate that failure locally, i think it has something to do with the test file system

@Arkatufus
Copy link
Contributor Author

Oh, it was this problem all over again: CoreyKaylor/Lightning.NET#104

@@ -5,6 +5,7 @@
<PropertyGroup>
<AssemblyTitle>Akka.DistributedData.Tests</AssemblyTitle>
<TargetFrameworks>$(NetFrameworkTestVersion);$(NetTestVersion);$(NetCoreTestVersion)</TargetFrameworks>
<PlatformTarget>x64</PlatformTarget>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to add a comment explaining why this is necessary - feel free to add it in a new PR @Arkatufus

Copy link
Member

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Aaronontheweb Aaronontheweb merged commit 0619316 into akkadotnet:dev Dec 13, 2021
@Aaronontheweb Aaronontheweb mentioned this pull request Dec 13, 2021
@Arkatufus Arkatufus deleted the #5423_LMDB_directory_crash branch December 13, 2021 21:10
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 this pull request may close these issues.

[DData] LMDB throws a MDB_NOTFOUND exception when the database directory already exists
2 participants