The attached patch tries to fix https://fedorahosted.org/freeipa/ticket/4378

After discussion with Thierry we concluded that while this issue is more complex than it seems, the transition from REPLACE to DEL/ADD operations when updating nsDS5ReplicaId should suffice for this ticket.

--
Martin^3 Babinsky
From 393e0041f67c4dc97577e9f2223d0bcd9ec7ad64 Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Mon, 4 May 2015 18:33:44 +0200
Subject: [PATCH] prevent duplicate IDs when setting up multiple replicas
 against single master

This patch forces replicas to use DELETE+ADD operations to increment
'nsDS5ReplicaId' in 'cn=replication,cn=etc,$SUFFIX' on master, and retry
multiple times in the case of conflict with another update. Thus when multiple
replicas are set-up against single master none of them will have duplicate ID.

https://fedorahosted.org/freeipa/ticket/4378
---
 ipaserver/install/replication.py | 78 ++++++++++++++++++++++++++--------------
 1 file changed, 52 insertions(+), 26 deletions(-)

diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index 66764c22f69328942fe2e4581cfafb3806438d7c..0b9f18f08d13540b9779b36479270d508a50a113 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -21,6 +21,7 @@ import time
 import datetime
 import sys
 import os
+from random import randint
 
 import ldap
 
@@ -230,34 +231,59 @@ class ReplicationManager(object):
 
         # Ok, either the entry doesn't exist or the attribute isn't set
         # so get it from the other master
-        retval = -1
+        return self._get_and_update_id_from_master(master_conn)
+
+    def _get_and_update_id_from_master(self, master_conn, attempts=5):
+        """
+        Fetch replica ID from remote master and update nsDS5ReplicaId attribute
+        on 'cn=replication,cn=etc,$SUFFIX' entry. Do it as MOD_DELETE+MOD_ADD
+        operations and retry when conflict occurs, e.g. due to simultaneous
+        update from another replica.
+        :param master_conn: LDAP connection to master
+        :param attempts: number of attempts to update nsDS5ReplicaId
+        :return: value of nsDS5ReplicaId before incrementation
+        """
         dn = DN(('cn','replication'),('cn','etc'), self.suffix)
-        try:
-            replica = master_conn.get_entry(dn)
-        except errors.NotFound:
-            root_logger.debug("Unable to retrieve nsDS5ReplicaId from remote server")
-            raise
-        else:
-            id_values = replica.get('nsDS5ReplicaId')
-            if not id_values:
+
+        for a in range(1, attempts + 1):
+            try:
+                root_logger.debug('Fetching nsDS5ReplicaId from master '
+                                  '[Attempt %d/%d]', a, attempts)
+                replica = master_conn.get_entry(dn)
+                id_values = replica.get('nsDS5ReplicaId')
+                if not id_values:
+                    root_logger.debug("Unable to retrieve nsDS5ReplicaId from remote server")
+                    raise RuntimeError("Unable to retrieve nsDS5ReplicaId from remote server")
+                # nsDS5ReplicaId is single-valued now, but historically it could
+                # contain multiple values, of which we need the highest.
+                # see bug: https://fedorahosted.org/freeipa/ticket/3394
+                retval = max(int(v) for v in id_values)
+
+                # Now update the value on the master
+                mod_list = [(ldap.MOD_DELETE, 'nsDS5ReplicaId', None),
+                            (ldap.MOD_ADD, 'nsDS5ReplicaId', str(retval + 1))]
+
+                master_conn.modify_s(dn, mod_list)
+                root_logger.debug('Successfully updated nsDS5ReplicaId.')
+                return retval
+
+            except errors.NotFound:
                 root_logger.debug("Unable to retrieve nsDS5ReplicaId from remote server")
-                raise RuntimeError("Unable to retrieve nsDS5ReplicaId from remote server")
-
-        # nsDS5ReplicaId is single-valued now, but historically it could
-        # contain multiple values, of which we need the highest.
-        # see bug: https://fedorahosted.org/freeipa/ticket/3394
-        retval = max(int(v) for v in id_values)
-
-        # Now update the value on the master
-        mod = [(ldap.MOD_REPLACE, 'nsDS5ReplicaId', str(retval + 1))]
-
-        try:
-            master_conn.modify_s(dn, mod)
-        except Exception, e:
-            root_logger.debug("Problem updating nsDS5ReplicaID %s" % e)
-            raise
-
-        return retval
+                raise
+            # these errors signal a conflict in updating replica ID.
+            # We then wait for a random time interval and try again
+            except (ldap.NO_SUCH_ATTRIBUTE, ldap.OBJECT_CLASS_VIOLATION) as e:
+                sleep_interval = randint(1, 5)
+                root_logger.debug("Update failed (%s). Conflicting operation?",
+                                  e)
+                time.sleep(sleep_interval)
+            # in case of other error we bail out
+            except ldap.LDAPError as e:
+                root_logger.debug("Problem updating nsDS5ReplicaID %s" % e)
+                raise
+
+        raise RuntimeError("Failed to update nsDS5ReplicaId in %d attempts"
+                           % attempts)
 
     def get_agreement_filter(self, agreement_types=None, host=None):
         """
-- 
2.1.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to