Skip to content

Issue 6827: Backports commit that regression test test_suffix_entryid is dependent on. - #6828

Open
abeisemb wants to merge 2 commits into
389ds:389-ds-base-2.4from
abeisemb:389-ds-base-2.4
Open

Issue 6827: Backports commit that regression test test_suffix_entryid is dependent on. #6828
abeisemb wants to merge 2 commits into
389ds:389-ds-base-2.4from
abeisemb:389-ds-base-2.4

Conversation

@abeisemb

@abeisemb abeisemb commented Jun 24, 2025 •
edited by sourcery-ai Bot
Loading

Copy link
Copy Markdown
Collaborator

Commit in question is f75b5e2, meant to resolve issue #6265.

Resolves: #6827

Summary by Sourcery

Isolate multi-master replication regression tests by using temporary credentials and restoring each topology after execution.

Bug Fixes:

  • Prevent replication regression tests from depending on group-based replication credentials that may be removed during test execution.

Enhancements:

  • Add a fixture that backs up the multi-master topology, assigns temporary dedicated replication credentials, and restores the topology after each test.

Tests:

  • Improve replication regression test isolation by preserving and restoring topology state around individual tests.

tbordaz
tbordaz reviewed Jun 25, 2025
View reviewed changes

@tbordaz tbordaz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Parts of the patch are related to LMDB. Do we need those parts in 2.4 ?

Comment thread ldap/servers/plugins/replication/repl5_agmt.c Outdated
#include "cl5_api.h"
#include "slapi-plugin.h"
#include "slap.h"
#include "../../slapd/back-ldbm/dbimpl.h" /* for dblayer_is_lmdb */

@tbordaz tbordaz Jun 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lmdb definitions are not expected in 2.4

Comment thread ldap/servers/plugins/replication/repl5_agmt.c Outdated
Slapi_Backend *be = NULL;
const char *val = NULL;
int64_t ptimeout = 0;
int use_lmdb = 0;

@tbordaz tbordaz Jun 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is no lmdb switch in 2.4

Comment thread ldap/servers/plugins/replication/repl5_agmt.c Outdated

/* flow control update window. */
ra->flowControlWindow = DEFAULT_FLOWCONTROL_WINDOW;
use_lmdb = dblayer_is_lmdb(be);

@tbordaz tbordaz Jun 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it available in 2.4 ?

Comment thread ldap/servers/slapd/back-ldbm/idl_new.c Outdated
return idl;
}

/*

@tbordaz tbordaz Jun 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I doubt those functions are needed in 2.4 as they are related to LMDB

tbordaz
tbordaz reviewed Jun 25, 2025
View reviewed changes

@tbordaz tbordaz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If the tests fail because of missing fixture. Is it enough to backport the fixture in 2.4 in the tests but leave unchanged the core server ?

@abeisemb

abeisemb commented Jun 25, 2025

Copy link
Copy Markdown
Collaborator Author

The test succeeds with just the fixture backported; I tested it manually previously. If the fixture alone is enough, I can amend this commit and remove the rest of it.

@abeisemb
abeisemb force-pushed the 389-ds-base-2.4 branch 2 times, most recently from d394190 to cdfdb73 Compare June 26, 2025 13:08
@abeisemb

abeisemb commented Jun 26, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Removed all changes from commit other than the new fixture. Regression test confirmed successful.
Files reset to base repo 389-ds-base-2.4:

  • ldap/servers/plugins/replication/repl5_agmt.c
  • ldap/servers/plugins/replication/repl5_connection.c
  • ldap/servers/slapd/back-ldbm/db-mdb/mdb_import_threads.c
  • ldap/servers/slapd/back-ldbm/idl_new.c
  • ldap/servers/slapd/back-ldbm/index.c
  • ldap/servers/slapd/back-ldbm/proto-back-ldbm.h

Edit: Forgot to mention this, but I also kept four imports necessary for the test and the fixture.

@abeisemb
abeisemb requested a review from tbordaz June 26, 2025 13:34
tbordaz
tbordaz approved these changes Jun 26, 2025
View reviewed changes

@tbordaz tbordaz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes LGTM.

@aadhikar

aadhikar commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

@abeisemb Hey, can you please rebase? Thanks!

@progier389 @aadhikar
Issue 6265 - lmdb - missing entries in range searches (389ds#6266)
d35928c 
* Issue 6265 - lmdb - missing entries in range searches

Several issues seen after generating ldif with 2000 users and importing it in a replica:

1. The entryid attribute in missing in the suffix entry.
2. Access log shows that the internal search looking for "(parentid>=1)" is not returning all entries but one.
3. When initializing a replica through a replication agreement some entries are missing (because of 2)
4. Once 2. get fixed, the bulk import still fails because the default values for nsds5ReplicaFlowControlWindow and nsds5ReplicaFlowControlPause are not adapted to lmdb (supplier sent the entry faster than bdb and the target replica import them slower.

The fix is about:

1. Ensuring that the operational attribute are properly set when importing the suffix entry.
2. and 3. Avoid using database bulk operation when computing range unless we are sure that bdb is used. (rely instead on the  generic dblayer database iterator - dblayer_cursor_iterate.
4. Change the default values for nsds5ReplicaFlowControlWindow and nsds5ReplicaFlowControlPause if agreement is on a lmdb backend.

Issue: 389ds#6265

Reviewed by: @vashirov, @droideck (Thanks!)
@aadhikar
aadhikar requested a review from tbordaz August 14, 2026 12:00
@aadhikar

aadhikar commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

@tbordaz Hi mate, can you please re-review this once?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tbordaz tbordaz Awaiting requested review from tbordaz

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@abeisemb @aadhikar @tbordaz @progier389 @vashirov