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 6159 - Add a test to check URP add and delete conflict #6160

Merged
merged 2 commits into from
May 27, 2024

Conversation

progier389
Copy link
Contributor

@progier389 progier389 commented Apr 30, 2024

Add URP tests that run if URP_VERY_LONG_TEST environment variable is set One test spends 6 days and check the 5770 different way of running the (Add, sync agmt 1, sync agmt 2, Del) sequence on 3 suppliers and check that when everything is in sync, the entries are the same everywhere Second test generate crossed entries and conflict entries
(In theory that should not happen but we have sometime seen them)
And tries to remove one of the entry.
Then once everything is back in sync, it check that the entry are the same
The second test fails - Apparently there is a problem in that corner case

Issue: #6159

Reviewed by: @droideck (Thanks!)

Add URP tests that run if URP_VERY_LONG_TEST environment variable is set
One test spends 6 days and check the 5770 different way of running
the (Add, sync agmt 1, sync agmt 2, Del) sequence on 3 suppliers
and check that when everything is in sync, the entries are the same everywhere
Second test generate crossed entries and conflict entries
 (In theory that should not happen but we have sometime seen them)
And tries to remove one of the entry.
Then once everything is back in sync, it check that the entry are the same
 The second test fails - Apparently there is a problem in that corner case
@progier389 progier389 added this to the 3.0.0 milestone Apr 30, 2024
def resync_agmt(self, instfrom, instto):
log.info(f"Enabling replication agreement from {instfrom.serverid} to {instto.serverid}")
self.repl.enable_to_supplier(instto, [instfrom,])
self.repl.wait_for_replication(instfrom, instto, timeout=REPL_SYNC_TIMEOUT)
Copy link
Member

Choose a reason for hiding this comment

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

It probably depends on the machine (I had a pretty slow VM) but I got at least 3 failures in test_urp_delete here with Replication from ldap://inst3:39003 to ldap://inst2:39002 is NOT working. (too many retries)

Probably, it's expected here or we can tune it a bit? (or make it flaky with some retries).
Still, the test work and looks good as it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On a relatively fast machine, the test is already needing 6 days so IMHO it is not realistic to
perform retries or run on slow machine. But I could add comment that the test should run on a fast machine and require memory (as there is 3 instances running even if the db are tiny )

Tasks(s1).importLDIF(DEFAULT_SUFFIX, input_file=import_ldif1, args={TASK_WAIT: True})
Tasks(s2).importLDIF(DEFAULT_SUFFIX, input_file=import_ldif2, args={TASK_WAIT: True})
# 3. Remove a conflict entry
ConflictEntry(s1, 'cn=u22449+nsUniqueId=c6654281-f11b11ee-ad93a02d-7ba2db25,dc=example,dc=com').delete()
Copy link
Member

@droideck droideck Apr 30, 2024

Choose a reason for hiding this comment

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

I have a failure here, though:

E ldap.OPERATIONS_ERROR: {'msgtype': 107, 'msgid': 88, 'result': 1, 'desc': 'Operations error', 'ctrls': [], 'ldap_request': "delete_ext_s(('cn=u22449+nsUniqueId=c6654281-f11b11ee-ad93a02d-7ba2db25,dc=example,dc=com',),{'serverctrls': None, 'clientctrls': None, 'escapehatch': 'i am sure'}) on instance supplier1"}

When I run it standalone:

[root@inst ds]# URP_VERY_LONG_TEST=yes py.test-3 -s -v dirsrvtests/tests/suites/replication/urp.py::test_urp_with_crossed_entries

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although I know that this test is failing, your failure is unexpected:
I have a different behavior on my machine:

URP_VERY_LONG_TEST=yes py.test-3 -s -v $PWD/urp.py::test_urp_with_crossed_entries
...
>       assert u1 == u2
E       AssertionError: assert ['regular:cd8...02d-7ba2db25'] == ['conflict:cd...02d-7ba2db25']
...

Probably worth to try to understand why we get different results

Copy link
Member

@droideck droideck May 23, 2024

Choose a reason for hiding this comment

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

Rechecked, and I got the same error as you now:

        u1 = search_entries(s1, 'cn=u22449')
        u2 = search_entries(s2, 'cn=u22449')
>       assert u1 == u2
E       AssertionError: assert ['regular:cd8...02d-7ba2db25'] == ['conflict:cd...02d-7ba2db25']

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, this time it is the expected result. URP does not properly handle this case
BTW, I should add a @pytest.mark.xfail for this one so it will be explicit.

@progier389
Copy link
Contributor Author

Added a new commit to:

  1. Add a comment about using reasonably fast machine
  2. Assert that import tasks were successful
  3. Set ldif file permission so that they are readable by everyone (dirsrv)

The permission issue was the reason why the tests results are different when using standard package compared to my environment (The import failed and the db was empty)

@progier389
Copy link
Contributor Author

Added the xfail, now the test result is
urp.py::test_urp_with_crossed_entries XFAIL (URP does not properly handle this case) [100%]

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.

LGTM

@progier389 progier389 merged commit 1a7abef into 389ds:main May 27, 2024
178 of 195 checks passed
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.

2 participants