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

Issue 6022 - lmdb inconsistency between vlv index and vlv cache names #6026

Merged
merged 3 commits into from
Jan 10, 2024

Conversation

progier389
Copy link
Contributor

@progier389 progier389 commented Jan 5, 2024

Problem: dbstat -L shows two vlv cache db for a single vlv index db.
There should only have a single one.

Fix:
Added a CI Test
Using a single dbmdb_recno_cache_get_dbname function to get the cache db name.
Fix dbmdb_build_dbname to also append the backend name if the name is a vlv cache

Also fixed some issue found while creating the CI test:
Fixed an error message that puzzled me to make it clearer.
Fixed a race condition in lmdb bulk import that logged crappy data in error logs and crashed the CI tests.

Issue: #6022

Reviewed by: @droideck (Thanks !)

@progier389 progier389 self-assigned this Jan 5, 2024
@progier389 progier389 added lmdb LMDB related work in progress Work in Progress - can be reviewed, but not ready for merge. labels Jan 5, 2024
@progier389
Copy link
Contributor Author

Got a nasty surprise: Also the test is passing as a single test but test fails when all the vlv tests are run.
First issue was that a simple cleanup was needed to remove data from previous testcase
but once that fixed the test is still failing: now some of the entries returned by the vlv are not the expected one.
==> review is on hold until that issue is solved.

@progier389 progier389 removed the work in progress Work in Progress - can be reviewed, but not ready for merge. label Jan 8, 2024
@progier389
Copy link
Contributor Author

I discovered that the vlv inconstancy I described in previews comments also exists with bdb so it is not related to the current issue. Created issue #6028 to track it. Restart works around that issue.
So it is ok to review.

Copy link
Member

@droideck droideck left a comment

Choose a reason for hiding this comment

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

Besides the minor issue, looks good!

entry.delete()
repl.wait_for_replication(M1, M2)
# Restart the instance to workaround github issue #6028
M2.restart()
Copy link
Member

Choose a reason for hiding this comment

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

If I understood correctly, the workaround is required only for BDB. Should we restart only for BDB tests, then? (by adding some if here or something)

Also, kind of nitpick, I think it'll be useful to have a full link here (https://github.com/389ds/389-ds-base/issues/6028) in case we'll change the bug tracker somewhere in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The new testcase is still failing, and I realized that I made a mistake when creating issue 6028
(used wrong test case in the reproducer)
Redoing the test I noticed that the inconsistency occurs only in mdb and the hang only in bdb
So I fixed 6028 description for mdb inconsistency and created #6029 for the bdb hang.
I changed the code to do as you suggested.
use full link to #6029 and restart M2 only in bdb case

Note the new test case is still failing with mdb but fixing 6028 should solve it.

@progier389 progier389 merged commit 04a2de9 into 389ds:main Jan 10, 2024
189 of 195 checks passed
@progier389 progier389 deleted the i6022 branch January 10, 2024 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lmdb LMDB related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lmdb inconsistency between vlv index sub-database name and vlv cache sub-database name
2 participants