[CalendarServer-changes] [11399] CalendarServer/trunk/txdav

source_changes at macosforge.org source_changes at macosforge.org
Tue Jun 25 07:08:23 PDT 2013


Revision: 11399
          http://trac.calendarserver.org//changeset/11399
Author:   cdaboo at apple.com
Date:     2013-06-25 07:08:23 -0700 (Tue, 25 Jun 2013)
Log Message:
-----------
Fix issue with deadlock on common home child revisions table change.

Modified Paths:
--------------
    CalendarServer/trunk/txdav/caldav/datastore/sql.py
    CalendarServer/trunk/txdav/caldav/datastore/test/test_sql.py
    CalendarServer/trunk/txdav/carddav/datastore/sql.py
    CalendarServer/trunk/txdav/carddav/datastore/test/common.py
    CalendarServer/trunk/txdav/carddav/datastore/test/test_sql.py
    CalendarServer/trunk/txdav/common/datastore/sql.py

Modified: CalendarServer/trunk/txdav/caldav/datastore/sql.py
===================================================================
--- CalendarServer/trunk/txdav/caldav/datastore/sql.py	2013-06-25 13:48:14 UTC (rev 11398)
+++ CalendarServer/trunk/txdav/caldav/datastore/sql.py	2013-06-25 14:08:23 UTC (rev 11399)
@@ -15,9 +15,6 @@
 # limitations under the License.
 ##
 
-from twext.enterprise.locking import NamedLock
-from urlparse import urlparse, urlunparse
-from txdav.caldav.datastore.scheduling.implicit import ImplicitScheduler
 
 """
 SQL backend for CalDAV storage.
@@ -36,9 +33,8 @@
 from twext.enterprise.dal.syntax import Select, Count, ColumnSyntax
 from twext.enterprise.dal.syntax import Update
 from twext.enterprise.dal.syntax import utcNowSQL
-
+from twext.enterprise.locking import NamedLock
 from twext.enterprise.util import parseSQLTimestamp
-
 from twext.python.clsprop import classproperty
 from twext.python.filepath import CachingFilePath
 from twext.python.log import Logger
@@ -49,6 +45,7 @@
 from twisted.internet.defer import inlineCallbacks, returnValue, succeed
 from twisted.python import hashlib
 
+
 from twistedcaldav import caldavxml, customxml
 from twistedcaldav.config import config
 from twistedcaldav.datafilters.peruserdata import PerUserDataFilter
@@ -57,8 +54,8 @@
 from twistedcaldav.ical import Component, InvalidICalendarDataError, Property
 from twistedcaldav.instance import InvalidOverriddenInstanceError
 from twistedcaldav.memcacher import Memcacher
-
 from txdav.base.propertystore.base import PropertyName
+from txdav.caldav.datastore.scheduling.implicit import ImplicitScheduler
 from txdav.caldav.datastore.util import AttachmentRetrievalTransport, \
     normalizationLookup
 from txdav.caldav.datastore.util import CalendarObjectBase
@@ -96,6 +93,7 @@
 
 from zope.interface.declarations import implements
 
+from urlparse import urlparse, urlunparse
 import collections
 import os
 import tempfile
@@ -706,7 +704,7 @@
         # CalDAV stores the default calendar properties on the inbox so we also need to send a changed notification on that
         inbox = (yield self.calendarWithName("inbox"))
         if inbox is not None:
-            yield inbox.notifyChanged()
+            yield inbox.notifyPropertyChanged()
 
 
     @inlineCallbacks
@@ -859,7 +857,12 @@
         yield self.invalidateQueryCache()
         yield self.notifyChanged()
 
+        # CalDAV stores the availability properties on the inbox so we also need to send a changed notification on that
+        inbox = (yield self.calendarWithName("inbox"))
+        if inbox is not None:
+            yield inbox.notifyPropertyChanged()
 
+
 CalendarHome._register(ECALENDARTYPE)
 
 
@@ -1040,7 +1043,7 @@
         ).on(self._txn)
         self._supportedComponents = supported_components
         yield self.invalidateQueryCache()
-        yield self.notifyChanged()
+        yield self.notifyPropertyChanged()
 
 
     def getTimezone(self):
@@ -1073,7 +1076,7 @@
             Where=(cal.CALENDAR_HOME_RESOURCE_ID == self.viewerHome()._resourceID).And(cal.CALENDAR_RESOURCE_ID == self._resourceID)
         ).on(self._txn)
         yield self.invalidateQueryCache()
-        yield self.notifyChanged()
+        yield self.notifyPropertyChanged()
 
     ALARM_DETAILS = {
         (True, True): (_bindSchema.ALARM_VEVENT_TIMED, "_alarm_vevent_timed"),
@@ -1120,7 +1123,7 @@
             Where=(cal.CALENDAR_HOME_RESOURCE_ID == self.viewerHome()._resourceID).And(cal.CALENDAR_RESOURCE_ID == self._resourceID)
         ).on(self._txn)
         yield self.invalidateQueryCache()
-        yield self.notifyChanged()
+        yield self.notifyPropertyChanged()
 
 
     def isInbox(self):
@@ -1160,7 +1163,7 @@
             Where=(cal.CALENDAR_HOME_RESOURCE_ID == self.viewerHome()._resourceID).And(cal.CALENDAR_RESOURCE_ID == self._resourceID)
         ).on(self._txn)
         yield self.invalidateQueryCache()
-        yield self.notifyChanged()
+        yield self.notifyPropertyChanged()
 
 
     def initPropertyStore(self, props):

Modified: CalendarServer/trunk/txdav/caldav/datastore/test/test_sql.py
===================================================================
--- CalendarServer/trunk/txdav/caldav/datastore/test/test_sql.py	2013-06-25 13:48:14 UTC (rev 11398)
+++ CalendarServer/trunk/txdav/caldav/datastore/test/test_sql.py	2013-06-25 14:08:23 UTC (rev 11399)
@@ -26,7 +26,7 @@
 from twext.python.vcomponent import VComponent
 
 from twisted.internet import reactor
-from twisted.internet.defer import inlineCallbacks, returnValue
+from twisted.internet.defer import inlineCallbacks, returnValue, DeferredList
 from twisted.internet.task import deferLater
 from twisted.trial import unittest
 
@@ -607,7 +607,7 @@
             yield cal1.createObjectResourceWithName("1.ics", VComponent.fromString(
 """BEGIN:VCALENDAR
 VERSION:2.0
-PRODID:-//Apple Inc.//iCal 4.0.1//EN
+PRODID:-//CALENDARSERVER.ORG//NONSGML Version 1//EN
 CALSCALE:GREGORIAN
 BEGIN:VTIMEZONE
 TZID:US/Pacific
@@ -653,7 +653,7 @@
             yield cal2.createObjectResourceWithName("2.ics", VComponent.fromString(
 """BEGIN:VCALENDAR
 VERSION:2.0
-PRODID:-//Apple Inc.//iCal 4.0.1//EN
+PRODID:-//CALENDARSERVER.ORG//NONSGML Version 1//EN
 CALSCALE:GREGORIAN
 BEGIN:VTIMEZONE
 TZID:US/Pacific
@@ -1557,7 +1557,7 @@
     @inlineCallbacks
     def test_objectResourceWithID(self):
         """
-        L{ICalendarHome.objectResourceWithID} will return the calendar object..
+        L{ICalendarHome.objectResourceWithID} will return the calendar object.
         """
         home = yield self.homeUnderTest()
         calendarObject = (yield home.objectResourceWithID(9999))
@@ -1571,7 +1571,7 @@
     @inlineCallbacks
     def test_defaultAlarms(self):
         """
-        L{ICalendarHome.objectResourceWithID} will return the calendar object..
+        L{ICalendarHome.objectResourceWithID} will return the calendar object.
         """
 
         alarmhome1 = """BEGIN:VALARM
@@ -1859,3 +1859,80 @@
         cal = yield self.calendarUnderTest()
         self.assertEqual(cal.getTimezone(), None)
         yield self.commit()
+
+
+    @inlineCallbacks
+    def test_calendarRevisionChangeConcurrency(self):
+        """
+        Test that two concurrent attempts to add resources in two separate
+        calendar homes does not deadlock on the revision table update.
+        """
+
+        calendarStore = self._sqlCalendarStore
+
+        # Make sure homes are provisioned
+        txn = self.transactionUnderTest()
+        home_uid1 = yield txn.homeWithUID(ECALENDARTYPE, "user01", create=True)
+        home_uid2 = yield txn.homeWithUID(ECALENDARTYPE, "user02", create=True)
+        self.assertNotEqual(home_uid1, None)
+        self.assertNotEqual(home_uid2, None)
+        yield self.commit()
+
+        # Create first events in different calendar homes
+        txn1 = calendarStore.newTransaction()
+        txn2 = calendarStore.newTransaction()
+
+        calendar_uid1_in_txn1 = yield self.calendarUnderTest(txn1, "calendar", "user01")
+        calendar_uid2_in_txn2 = yield self.calendarUnderTest(txn2, "calendar", "user02")
+
+        data = """BEGIN:VCALENDAR
+VERSION:2.0
+CALSCALE:GREGORIAN
+PRODID:-//CALENDARSERVER.ORG//NONSGML Version 1//EN
+BEGIN:VEVENT
+UID:data%(ctr)s
+DTSTART:20130102T140000Z
+DURATION:PT1H
+CREATED:20060102T190000Z
+DTSTAMP:20051222T210507Z
+SUMMARY:data%(ctr)s
+END:VEVENT
+END:VCALENDAR
+"""
+
+        component = Component.fromString(data % {"ctr": 1})
+        yield calendar_uid1_in_txn1.createCalendarObjectWithName("data1.ics", component)
+
+        component = Component.fromString(data % {"ctr": 2})
+        yield calendar_uid2_in_txn2.createCalendarObjectWithName("data2.ics", component)
+
+        # Setup deferreds to run concurrently and create second events in the calendar homes
+        # previously used by the other transaction - this could create the deadlock.
+        @inlineCallbacks
+        def _defer_uid3():
+            calendar_uid1_in_txn2 = yield self.calendarUnderTest(txn2, "calendar", "user01")
+            component = Component.fromString(data % {"ctr": 3})
+            yield calendar_uid1_in_txn2.createCalendarObjectWithName("data3.ics", component)
+            yield txn2.commit()
+        d1 = _defer_uid3()
+
+        @inlineCallbacks
+        def _defer_uid4():
+            calendar_uid2_in_txn1 = yield self.calendarUnderTest(txn1, "calendar", "user02")
+            component = Component.fromString(data % {"ctr": 4})
+            yield calendar_uid2_in_txn1.createCalendarObjectWithName("data4.ics", component)
+            yield txn1.commit()
+        d2 = _defer_uid4()
+
+        # Now do the concurrent provision attempt
+        yield DeferredList([d1, d2])
+
+        # Verify we did not have a deadlock and all resources have been created.
+        caldata1 = yield self.calendarObjectUnderTest(name="data1.ics", calendar_name="calendar", home="user01")
+        caldata2 = yield self.calendarObjectUnderTest(name="data2.ics", calendar_name="calendar", home="user02")
+        caldata3 = yield self.calendarObjectUnderTest(name="data3.ics", calendar_name="calendar", home="user01")
+        caldata4 = yield self.calendarObjectUnderTest(name="data4.ics", calendar_name="calendar", home="user02")
+        self.assertNotEqual(caldata1, None)
+        self.assertNotEqual(caldata2, None)
+        self.assertNotEqual(caldata3, None)
+        self.assertNotEqual(caldata4, None)

Modified: CalendarServer/trunk/txdav/carddav/datastore/sql.py
===================================================================
--- CalendarServer/trunk/txdav/carddav/datastore/sql.py	2013-06-25 13:48:14 UTC (rev 11398)
+++ CalendarServer/trunk/txdav/carddav/datastore/sql.py	2013-06-25 14:08:23 UTC (rev 11399)
@@ -25,8 +25,6 @@
     "AddressBookObject",
 ]
 
-from uuid import uuid4
-
 from twext.enterprise.dal.syntax import Delete, Insert, Len, Parameter, \
     Update, Union, Max, Select, utcNowSQL
 from twext.enterprise.locking import NamedLock
@@ -37,6 +35,7 @@
 
 from twisted.internet.defer import inlineCallbacks, returnValue
 from twisted.python import hashlib
+
 from twistedcaldav import carddavxml, customxml
 from twistedcaldav.config import config
 from twistedcaldav.memcacher import Memcacher
@@ -60,6 +59,8 @@
     InvalidObjectResourceError, InvalidComponentForStoreError, \
     AllRetriesFailed
 
+from uuid import uuid4
+
 from zope.interface.declarations import implements
 
 
@@ -100,7 +101,7 @@
 
 
     @classproperty
-    def _resourceIDAndHomeResourceIDFromOwnerQuery(cls):  #@NoSelf
+    def _resourceIDAndHomeResourceIDFromOwnerQuery(cls): #@NoSelf
         home = cls._homeSchema
         return Select([home.RESOURCE_ID, home.ADDRESSBOOK_PROPERTY_STORE_ID],
                       From=home, Where=home.OWNER_UID == Parameter("ownerUID"))
@@ -264,7 +265,7 @@
 
 
     @classproperty
-    def _syncTokenQuery(cls):  #@NoSelf
+    def _syncTokenQuery(cls): #@NoSelf
         """
         DAL Select statement to find the sync token.
         """
@@ -308,7 +309,7 @@
 
 
     @classproperty
-    def _changesQuery(cls):  #@NoSelf
+    def _changesQuery(cls): #@NoSelf
         rev = cls._revisionsSchema
         return Select(
             [rev.COLLECTION_NAME,
@@ -425,7 +426,7 @@
                 self.viewerHome().uid(),
                 self._txn,
                 self.ownerHome()._addressbookPropertyStoreID,  # not ._resourceID as in CommonHomeChild._loadPropertyStore()
-                notifyCallback=self.notifyChanged
+                notifyCallback=self.notifyPropertyChanged
             )
         super(AddressBook, self)._loadPropertyStore(props)
 
@@ -476,7 +477,7 @@
             yield self.properties()._removeResource()
             yield self._loadPropertyStore()
 
-            yield self.notifyChanged()
+            yield self.notifyPropertyChanged()
             yield self._home.notifyChanged()
         else:
             returnValue((yield super(AddressBook, self).remove()))
@@ -551,7 +552,7 @@
                       Where=obj.ADDRESSBOOK_HOME_RESOURCE_ID == Parameter("addressbookResourceID"),)
 
 
-    def _fullySharedAddressBookGroupRow(self):  #@NoSelf
+    def _fullySharedAddressBookGroupRow(self): #@NoSelf
         return [
             self._resourceID,  # obj.ADDRESSBOOK_HOME_RESOURCE_ID,
             self._resourceID,  # obj.RESOURCE_ID,
@@ -646,7 +647,7 @@
         )
         # get ownerHomeIDs
         for dataRow in dataRows:
-            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = dataRow[:cls.bindColumnCount]  #@UnusedVariable
+            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = dataRow[:cls.bindColumnCount] #@UnusedVariable
             ownerHome = yield home.ownerHomeWithChildID(resourceID)
             ownerHomeToDataRowMap[ownerHome] = dataRow
 
@@ -655,7 +656,7 @@
             home._txn, homeID=home._resourceID
         )
         for groupBindRow in groupBindRows:
-            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupBindRow[:cls.bindColumnCount]  #@UnusedVariable
+            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupBindRow[:cls.bindColumnCount] #@UnusedVariable
             ownerAddressBookID = yield AddressBookObject.ownerAddressBookIDFromGroupID(home._txn, resourceID)
             ownerHome = yield home.ownerHomeWithChildID(ownerAddressBookID)
             if ownerHome not in ownerHomeToDataRowMap:
@@ -678,7 +679,7 @@
 
             # Create the actual objects merging in properties
             for ownerHome, dataRow in ownerHomeToDataRowMap.iteritems():
-                bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = dataRow[:cls.bindColumnCount]  #@UnusedVariable
+                bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = dataRow[:cls.bindColumnCount] #@UnusedVariable
                 additionalBind = dataRow[cls.bindColumnCount:cls.bindColumnCount + len(cls.additionalBindColumns())]
                 metadata = dataRow[cls.bindColumnCount + len(cls.additionalBindColumns()):]
 
@@ -772,7 +773,7 @@
         if not rows:
             returnValue(None)
 
-        bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage, ownerAddressBookID, cachedBindStatus = rows[0]  #@UnusedVariable
+        bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage, ownerAddressBookID, cachedBindStatus = rows[0] #@UnusedVariable
 
         # if wrong status, exit here.  Item is in queryCache
         if (cachedBindStatus == _BIND_STATUS_ACCEPTED) != bool(accepted):
@@ -808,7 +809,7 @@
         """
         bindRows = yield cls._bindForNameAndHomeID.on(home._txn, name=name, homeID=home._resourceID)
         if bindRows:
-            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = bindRows[0]  #@UnusedVariable
+            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = bindRows[0] #@UnusedVariable
             if (bindStatus == _BIND_STATUS_ACCEPTED) != bool(accepted):
                 returnValue(None)
 
@@ -824,7 +825,7 @@
             home._txn, name=name, homeID=home._resourceID
         )
         if groupBindRows:
-            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupBindRows[0]  #@UnusedVariable
+            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupBindRows[0] #@UnusedVariable
             if (bindStatus == _BIND_STATUS_ACCEPTED) != bool(accepted):
                 returnValue(None)
 
@@ -865,7 +866,7 @@
             home._txn, resourceID=resourceID, homeID=home._resourceID
         )
         if bindRows:
-            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = bindRows[0]  #@UnusedVariable
+            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = bindRows[0] #@UnusedVariable
             if (bindStatus == _BIND_STATUS_ACCEPTED) != bool(accepted):
                 returnValue(None)
 
@@ -879,7 +880,7 @@
                     home._txn, homeID=home._resourceID, addressbookID=resourceID
         )
         if groupBindRows:
-            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupBindRows[0]  #@UnusedVariable
+            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupBindRows[0] #@UnusedVariable
             if (bindStatus == _BIND_STATUS_ACCEPTED) != bool(accepted):
                 returnValue(None)
 
@@ -928,7 +929,7 @@
         ).on(self._txn, resourceID=self._resourceID, homeID=self.viewerHome()._resourceID)
 
         yield self.invalidateQueryCache()
-        yield self.notifyChanged()
+        yield self.notifyPropertyChanged()
         '''
         yield None
 
@@ -947,7 +948,7 @@
             home._txn, homeID=home._resourceID
         )
         for row in rows:
-            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = row[:cls.bindColumnCount]  #@UnusedVariable
+            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = row[:cls.bindColumnCount] #@UnusedVariable
             ownerHome = yield home._txn.homeWithResourceID(home._homeType, resourceID, create=True)
             names |= set([ownerHome.shareeAddressBookName()])
 
@@ -955,7 +956,7 @@
             home._txn, homeID=home._resourceID
         )
         for groupRow in groupRows:
-            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupRow[:AddressBookObject.bindColumnCount]  #@UnusedVariable
+            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupRow[:AddressBookObject.bindColumnCount] #@UnusedVariable
             ownerAddressBookID = yield AddressBookObject.ownerAddressBookIDFromGroupID(home._txn, resourceID)
             ownerHome = yield home._txn.homeWithResourceID(home._homeType, ownerAddressBookID, create=True)
             names |= set([ownerHome.shareeAddressBookName()])
@@ -1026,7 +1027,7 @@
             readWriteGroupIDs = []
             readOnlyGroupIDs = []
             for groupBindRow in groupBindRows:
-                bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupBindRow[:self.bindColumnCount]  #@UnusedVariable
+                bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupBindRow[:self.bindColumnCount] #@UnusedVariable
                 if bindMode == _BIND_MODE_WRITE:
                     readWriteGroupIDs.append(resourceID)
                 else:
@@ -1074,7 +1075,7 @@
         readWriteGroupIDs = []
         readOnlyGroupIDs = []
         for groupBindRow in groupBindRows:
-            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupBindRow[:self.bindColumnCount]  #@UnusedVariable
+            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupBindRow[:self.bindColumnCount] #@UnusedVariable
             if bindMode == _BIND_MODE_WRITE:
                 readWriteGroupIDs.append(resourceID)
             else:
@@ -1189,7 +1190,7 @@
             shareeView._name = sharedname[0][0]
 
             # Must send notification to ensure cache invalidation occurs
-            yield self.notifyChanged()
+            yield self.notifyPropertyChanged()
 
         returnValue(shareeView._name)
 
@@ -1213,7 +1214,7 @@
                 self._txn, resourceID=self._resourceID, homeID=self._home._resourceID
             )
             for bindRow in bindRows:
-                bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = bindRow[:self.bindColumnCount]  #@UnusedVariable
+                bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = bindRow[:self.bindColumnCount] #@UnusedVariable
                 home = yield self._txn.homeWithResourceID(self._home._homeType, homeID, create=True)
                 new = yield home.childWithName(self.shareeAddressBookName())
                 result.append(new)
@@ -1240,7 +1241,7 @@
                 self._txn, resourceID=self._resourceID
             )
             for bindRow in bindRows:
-                bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = bindRow[:self.bindColumnCount]  #@UnusedVariable
+                bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = bindRow[:self.bindColumnCount] #@UnusedVariable
                 home = yield self._txn.homeWithResourceID(self._home._homeType, homeID, create=True)
                 new = yield self.objectWithName(home, self.shareeAddressBookName(), accepted=False)
                 result.append(new)
@@ -1277,7 +1278,7 @@
                 self._objectNames = None
 
             # Must send notification to ensure cache invalidation occurs
-            yield self.notifyChanged()
+            yield self.notifyPropertyChanged()
 
         # delete binds including invites
         deletedBindNameRows = yield self._deleteBindForResourceIDAndHomeID.on(self._txn, resourceID=self._resourceID,
@@ -1308,7 +1309,7 @@
     #_homeChildMetaDataSchema = schema.ADDRESSBOOK_OBJECT
 
 
-    def __init__(self, addressbook, name, uid, resourceID=None, options=None):  #@UnusedVariable
+    def __init__(self, addressbook, name, uid, resourceID=None, options=None): #@UnusedVariable
 
         self._kind = None
         self._ownerAddressBookResourceID = None
@@ -1443,7 +1444,7 @@
 
 
     @classproperty
-    def _allColumnsWithResourceID(cls):  #@NoSelf
+    def _allColumnsWithResourceID(cls): #@NoSelf
         obj = cls._objectSchema
         return Select(
             cls._allColumns, From=obj,
@@ -1542,7 +1543,7 @@
 
                 if groupBindRows:
                     groupBindRow = groupBindRows[0]
-                    bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupBindRow[:self.bindColumnCount]  #@UnusedVariable
+                    bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupBindRow[:self.bindColumnCount] #@UnusedVariable
                     self._bindMode = bindMode
                     self._bindStatus = bindStatus
                     self._bindMessage = bindMessage
@@ -1559,7 +1560,7 @@
 
 
     @classproperty
-    def _allColumns(cls):  #@NoSelf
+    def _allColumns(cls): #@NoSelf
         """
         Full set of columns in the object table that need to be loaded to
         initialize the object resource state.
@@ -1673,7 +1674,7 @@
         self.validAddressDataCheck(component, inserting)
 
 
-    def validAddressDataCheck(self, component, inserting):  #@UnusedVariable
+    def validAddressDataCheck(self, component, inserting): #@UnusedVariable
         """
         Check that the calendar data is valid iCalendar.
         @return:         tuple: (True/False if the calendar data is valid,
@@ -1779,7 +1780,7 @@
 
 
     @classproperty
-    def _insertABObject(cls):  #@NoSelf
+    def _insertABObject(cls): #@NoSelf
         """
         DAL statement to create an addressbook object with all default values.
         """
@@ -1799,7 +1800,7 @@
 
 
     @inlineCallbacks
-    def updateDatabase(self, component, expand_until=None, reCreate=False,  #@UnusedVariable
+    def updateDatabase(self, component, expand_until=None, reCreate=False, #@UnusedVariable
                        inserting=False):
         """
         Update the database tables for the new data being written.
@@ -2150,7 +2151,7 @@
 
     # same as CommonHomeChild._childrenAndMetadataForHomeID() w/o metadata join
     @classproperty
-    def _childrenAndMetadataForHomeID(cls):  #@NoSelf
+    def _childrenAndMetadataForHomeID(cls): #@NoSelf
         bind = cls._bindSchema
         child = cls._objectSchema
         columns = cls.bindColumns() + cls.additionalBindColumns() + cls.metadataColumns()
@@ -2186,7 +2187,7 @@
                 self._txn, resourceID=self._resourceID, homeID=self._home._resourceID
             )
             for groupBindRow in groupBindRows:
-                bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupBindRow[:self.bindColumnCount]  #@UnusedVariable
+                bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupBindRow[:self.bindColumnCount] #@UnusedVariable
                 home = yield self._txn.homeWithResourceID(self._home._homeType, homeID, create=True)
                 addressbook = yield home.childWithName(self._home.shareeAddressBookName())
                 new = yield addressbook.objectResourceWithID(resourceID)
@@ -2215,7 +2216,7 @@
                 self._txn, resourceID=self._resourceID
             )
             for groupBindRow in groupBindRows:
-                bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupBindRow[:self.bindColumnCount]  #@UnusedVariable
+                bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupBindRow[:self.bindColumnCount] #@UnusedVariable
                 home = yield self._txn.homeWithResourceID(self._home._homeType, homeID, create=True)
                 addressbook = yield home.childWithName(self._home.shareeAddressBookName())
                 if not addressbook:
@@ -2227,7 +2228,7 @@
 
 
     @classproperty
-    def _addressbookIDForResourceID(cls):  #@NoSelf
+    def _addressbookIDForResourceID(cls): #@NoSelf
         obj = cls._objectSchema
         return Select([obj.PARENT_RESOURCE_ID],
                       From=obj,
@@ -2352,7 +2353,7 @@
                 self._txn, resourceID=self._resourceID, homeID=shareeHome._resourceID
             )
             groupBindRow = groupBindRows[0]
-            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupBindRow[:self.bindColumnCount]  #@UnusedVariable
+            bindMode, homeID, resourceID, bindName, bindStatus, bindRevision, bindMessage = groupBindRow[:self.bindColumnCount] #@UnusedVariable
             if bindStatus == _BIND_STATUS_ACCEPTED:
                 group = yield shareeHome.objectWithShareUID(bindName)
             else:
@@ -2465,7 +2466,7 @@
 
 
     @classproperty
-    def _acceptedBindForHomeIDAndAddressBookID(cls):  #@NoSelf
+    def _acceptedBindForHomeIDAndAddressBookID(cls): #@NoSelf
         bind = cls._bindSchema
         abo = cls._objectSchema
         return Select(
@@ -2479,7 +2480,7 @@
 
 
     @classproperty
-    def _unacceptedBindForHomeIDAndAddressBookID(cls):  #@NoSelf
+    def _unacceptedBindForHomeIDAndAddressBookID(cls): #@NoSelf
         bind = cls._bindSchema
         abo = cls._objectSchema
         return Select(
@@ -2493,7 +2494,7 @@
 
 
     @classproperty
-    def _bindForHomeIDAndAddressBookID(cls):  #@NoSelf
+    def _bindForHomeIDAndAddressBookID(cls): #@NoSelf
         bind = cls._bindSchema
         abo = cls._objectSchema
         return Select(

Modified: CalendarServer/trunk/txdav/carddav/datastore/test/common.py
===================================================================
--- CalendarServer/trunk/txdav/carddav/datastore/test/common.py	2013-06-25 13:48:14 UTC (rev 11398)
+++ CalendarServer/trunk/txdav/carddav/datastore/test/common.py	2013-06-25 14:08:23 UTC (rev 11399)
@@ -188,21 +188,21 @@
 
 
     @inlineCallbacks
-    def addressbookUnderTest(self, txn=None, name=None):
+    def addressbookUnderTest(self, txn=None, name=None, home="home1"):
         """
         Get the addressbook detailed by C{requirements['home1']['addressbook']}.
         """
-        returnValue((yield (yield self.homeUnderTest(txn=txn))
+        returnValue((yield (yield self.homeUnderTest(txn=txn, name=home))
             .addressbookWithName(name if name else "addressbook")))
 
 
     @inlineCallbacks
-    def addressbookObjectUnderTest(self, txn=None, name=None):
+    def addressbookObjectUnderTest(self, txn=None, name=None, addressbook_name="addressbook", home="home1"):
         """
         Get the addressbook detailed by
         C{requirements['home1']['addressbook']['1.vcf']}.
         """
-        returnValue((yield (yield self.addressbookUnderTest(txn=txn))
+        returnValue((yield (yield self.addressbookUnderTest(txn=txn, name=addressbook_name, home=home))
                     .addressbookObjectWithName(name if name else "1.vcf")))
 
 

Modified: CalendarServer/trunk/txdav/carddav/datastore/test/test_sql.py
===================================================================
--- CalendarServer/trunk/txdav/carddav/datastore/test/test_sql.py	2013-06-25 13:48:14 UTC (rev 11398)
+++ CalendarServer/trunk/txdav/carddav/datastore/test/test_sql.py	2013-06-25 14:08:23 UTC (rev 11399)
@@ -22,7 +22,7 @@
 from twext.enterprise.dal.syntax import Select, Parameter
 
 from twisted.internet import reactor
-from twisted.internet.defer import inlineCallbacks, returnValue
+from twisted.internet.defer import inlineCallbacks, returnValue, DeferredList
 from twisted.internet.task import deferLater
 
 from twisted.trial import unittest
@@ -472,7 +472,7 @@
         group = VCard.fromString(
             """BEGIN:VCARD
 VERSION:3.0
-PRODID:-//Apple Inc.//AddressBook 6.1//EN
+PRODID:-//CALENDARSERVER.ORG//NONSGML Version 1//EN
 UID:uid2
 FN:Top Group
 N:Top Group;;;;
@@ -489,7 +489,7 @@
         badgroup = VCard.fromString(
             """BEGIN:VCARD
 VERSION:3.0
-PRODID:-//Apple Inc.//AddressBook 6.1//EN
+PRODID:-//CALENDARSERVER.ORG//NONSGML Version 1//EN
 UID:uid3
 FN:Bad Group
 N:Bad Group;;;;
@@ -562,7 +562,7 @@
         group = VCard.fromString(
             """BEGIN:VCARD
 VERSION:3.0
-PRODID:-//Apple Inc.//AddressBook 6.1//EN
+PRODID:-//CALENDARSERVER.ORG//NONSGML Version 1//EN
 UID:uid2
 FN:Top Group
 N:Top Group;;;;
@@ -585,7 +585,7 @@
         subgroup = VCard.fromString(
             """BEGIN:VCARD
 VERSION:3.0
-PRODID:-//Apple Inc.//AddressBook 6.1//EN
+PRODID:-//CALENDARSERVER.ORG//NONSGML Version 1//EN
 UID:uid3
 FN:Sub Group
 N:Sub Group;;;;
@@ -949,3 +949,74 @@
             changed, deleted = yield otherHome.resourceNamesSinceRevision(otherAB._bindRevision, depth)
             self.assertEqual(len(changed), 0)
             self.assertEqual(len(deleted), 0)
+
+
+    @inlineCallbacks
+    def test_addressbookRevisionChangeConcurrency(self):
+        """
+        Test that two concurrent attempts to add resources in two separate
+        calendar homes does not deadlock on the revision table update.
+        """
+
+        # Make sure homes are provisioned
+        txn = self.transactionUnderTest()
+        home_uid1 = yield txn.homeWithUID(EADDRESSBOOKTYPE, "user01", create=True)
+        home_uid2 = yield txn.homeWithUID(EADDRESSBOOKTYPE, "user02", create=True)
+        self.assertNotEqual(home_uid1, None)
+        self.assertNotEqual(home_uid2, None)
+        yield self.commit()
+
+        # Create first events in different calendar homes
+        txn1 = self._sqlStore.newTransaction()
+        txn2 = self._sqlStore.newTransaction()
+
+        addressbook_uid1_in_txn1 = yield self.addressbookUnderTest(txn1, "addressbook", "user01")
+        addressbook_uid2_in_txn2 = yield self.addressbookUnderTest(txn2, "addressbook", "user02")
+
+        data = """BEGIN:VCARD
+VERSION:3.0
+PRODID:-//CALENDARSERVER.ORG//NONSGML Version 1//EN
+UID:data%(ctr)s
+FN:Data %(ctr)s
+N:Sub Group;;;;
+REV:20120503T194243Z
+END:VCARD
+
+"""
+
+        component = VComponent.fromString(data % {"ctr": 1})
+        yield addressbook_uid1_in_txn1.createAddressBookObjectWithName("data1.ics", component)
+
+        component = VComponent.fromString(data % {"ctr": 2})
+        yield addressbook_uid2_in_txn2.createAddressBookObjectWithName("data2.ics", component)
+
+        # Setup deferreds to run concurrently and create second events in the calendar homes
+        # previously used by the other transaction - this could create the deadlock.
+        @inlineCallbacks
+        def _defer_uid3():
+            addressbook_uid1_in_txn2 = yield self.addressbookUnderTest(txn2, "addressbook", "user01")
+            component = VComponent.fromString(data % {"ctr": 3})
+            yield addressbook_uid1_in_txn2.createAddressBookObjectWithName("data3.ics", component)
+            yield txn2.commit()
+        d1 = _defer_uid3()
+
+        @inlineCallbacks
+        def _defer_uid4():
+            addressbook_uid2_in_txn1 = yield self.addressbookUnderTest(txn1, "addressbook", "user02")
+            component = VComponent.fromString(data % {"ctr": 4})
+            yield addressbook_uid2_in_txn1.createAddressBookObjectWithName("data4.ics", component)
+            yield txn1.commit()
+        d2 = _defer_uid4()
+
+        # Now do the concurrent provision attempt
+        yield DeferredList([d1, d2])
+
+        # Verify we did not have a deadlock and all resources have been created.
+        vcarddata1 = yield self.addressbookObjectUnderTest(name="data1.ics", addressbook_name="addressbook", home="user01")
+        vcarddata2 = yield self.addressbookObjectUnderTest(name="data2.ics", addressbook_name="addressbook", home="user02")
+        vcarddata3 = yield self.addressbookObjectUnderTest(name="data3.ics", addressbook_name="addressbook", home="user01")
+        vcarddata4 = yield self.addressbookObjectUnderTest(name="data4.ics", addressbook_name="addressbook", home="user02")
+        self.assertNotEqual(vcarddata1, None)
+        self.assertNotEqual(vcarddata2, None)
+        self.assertNotEqual(vcarddata3, None)
+        self.assertNotEqual(vcarddata4, None)

Modified: CalendarServer/trunk/txdav/common/datastore/sql.py
===================================================================
--- CalendarServer/trunk/txdav/common/datastore/sql.py	2013-06-25 13:48:14 UTC (rev 11398)
+++ CalendarServer/trunk/txdav/common/datastore/sql.py	2013-06-25 14:08:23 UTC (rev 11399)
@@ -25,39 +25,44 @@
     "CommonHome",
 ]
 
-import sys
+from cStringIO import StringIO
 
-from uuid import uuid4, UUID
+from pycalendar.datetime import PyCalendarDateTime
 
-from zope.interface import implements, directlyProvides
-
+from twext.enterprise.dal.syntax import \
+    Delete, utcNowSQL, Union, Insert, Len, Max, Parameter, SavepointAction, \
+    Select, Update, ColumnSyntax, TableSyntax, Upper, Count, ALL_COLUMNS, Sum
+from twext.enterprise.ienterprise import AlreadyFinishedError
+from twext.enterprise.queue import LocalQueuer
+from twext.enterprise.util import parseSQLTimestamp
+from twext.internet.decorate import memoizedKey, Memoizable
+from twext.python.clsprop import classproperty
 from twext.python.log import Logger
-
-from txdav.xml.parser import WebDAVDocument
 from twext.web2.http_headers import MimeType
 
+from twisted.application.service import Service
+from twisted.internet import reactor
+from twisted.internet.defer import inlineCallbacks, returnValue, succeed
 from twisted.python import hashlib
+from twisted.python.failure import Failure
 from twisted.python.modules import getModule
 from twisted.python.util import FancyEqMixin
-from twisted.python.failure import Failure
 
-from twisted.internet import reactor
-from twisted.internet.defer import inlineCallbacks, returnValue, succeed
+from twistedcaldav.config import config
+from twistedcaldav.customxml import NotificationType
+from twistedcaldav.dateops import datetimeMktime, pyCalendarTodatetime
 
-from twisted.application.service import Service
-
 from txdav.base.datastore.util import QueryCacher
-
-from twext.internet.decorate import memoizedKey, Memoizable
-
+from txdav.base.datastore.util import normalizeUUIDOrNot
+from txdav.base.propertystore.none import PropertyStore as NonePropertyStore
+from txdav.base.propertystore.sql import PropertyStore
 from txdav.caldav.icalendarstore import ICalendarTransaction, ICalendarStore
-
 from txdav.carddav.iaddressbookstore import IAddressBookTransaction
-
 from txdav.common.datastore.common import HomeChildBase
-from txdav.common.datastore.sql_tables import schema, splitSQLString
 from txdav.common.datastore.sql_tables import _BIND_MODE_OWN, \
     _BIND_STATUS_ACCEPTED, _BIND_STATUS_DECLINED
+from txdav.common.datastore.sql_tables import schema, splitSQLString
+from txdav.common.icommondatastore import ConcurrentModification
 from txdav.common.icommondatastore import HomeChildNameNotAllowedError, \
     HomeChildNameAlreadyExistsError, NoSuchHomeChildError, \
     ObjectResourceNameNotAllowedError, ObjectResourceNameAlreadyExistsError, \
@@ -66,30 +71,13 @@
 from txdav.common.idirectoryservice import IStoreDirectoryService
 from txdav.common.inotifications import INotificationCollection, \
     INotificationObject
+from txdav.xml.parser import WebDAVDocument
 
-from twext.python.clsprop import classproperty
-from twext.enterprise.ienterprise import AlreadyFinishedError
+from uuid import uuid4, UUID
 
-from twext.enterprise.dal.syntax import \
-    Delete, utcNowSQL, Union, Insert, Len, Max, Parameter, SavepointAction, \
-    Select, Update, ColumnSyntax, TableSyntax, Upper, Count, ALL_COLUMNS, Sum
+from zope.interface import implements, directlyProvides
 
-from twistedcaldav.config import config
-
-from txdav.base.propertystore.none import PropertyStore as NonePropertyStore
-from txdav.base.propertystore.sql import PropertyStore
-
-from txdav.common.icommondatastore import ConcurrentModification
-from twistedcaldav.customxml import NotificationType
-from twistedcaldav.dateops import datetimeMktime, pyCalendarTodatetime
-
-from txdav.base.datastore.util import normalizeUUIDOrNot
-from twext.enterprise.queue import LocalQueuer
-from twext.enterprise.util import parseSQLTimestamp
-
-from pycalendar.datetime import PyCalendarDateTime
-
-from cStringIO import StringIO
+import sys
 import time
 
 current_sql_schema = getModule(__name__).filePath.sibling("sql_schema").child("current.sql").getContent()
@@ -1970,7 +1958,12 @@
                                 deleted.add("%s/%s" % (path, name,))
 
                 for path, name, wasdeleted in results:
-                    changed.add("%s/%s" % (path, "" if depth == "1" else name,))
+                    # Always report collection as changed
+                    changed.add("%s/" % (path,))
+                    if name:
+                        # Resource changed - for depth "infinity" report resource as changed
+                        if depth != "1":
+                            changed.add("%s/%s" % (path, name,))
 
         changed = sorted(changed)
         deleted = sorted(deleted)
@@ -2783,7 +2776,7 @@
                 yield shareeView._initBindRevision()
 
         # Must send notification to ensure cache invalidation occurs
-        yield self.notifyChanged()
+        yield self.notifyPropertyChanged()
         yield shareeHome.notifyChanged()
 
         returnValue(bindName)
@@ -2862,7 +2855,7 @@
             shareeView._name = sharedname[0][0]
 
             # Must send notification to ensure cache invalidation occurs
-            yield self.notifyChanged()
+            yield self.notifyPropertyChanged()
             yield shareeView.viewerHome().notifyChanged()
 
         returnValue(shareeView._name)
@@ -2890,7 +2883,7 @@
                 shareeHome._children.pop(shareeChild._resourceID, None)
 
                 # Must send notification to ensure cache invalidation occurs
-                yield self.notifyChanged()
+                yield self.notifyPropertyChanged()
                 yield shareeHome.notifyChanged()
                 break
 
@@ -3048,7 +3041,7 @@
         ).on(self._txn, resourceID=self._resourceID, homeID=self.viewerHome()._resourceID)
 
         yield self.invalidateQueryCache()
-        yield self.notifyChanged()
+        yield self.notifyPropertyChanged()
 
 
     def shareStatus(self):
@@ -3471,7 +3464,7 @@
 
         # Change notification for a create is on the home collection
         yield home.notifyChanged()
-        yield child.notifyChanged()
+        yield child.notifyPropertyChanged()
         returnValue(child)
 
 
@@ -3591,7 +3584,7 @@
         self._home._children[name] = self
         yield self._renameSyncToken()
 
-        yield self.notifyChanged()
+        yield self.notifyPropertyChanged()
         yield self._home.notifyChanged()
 
 
@@ -3608,7 +3601,7 @@
     def remove(self):
 
         # Do before setting _resourceID making changes
-        yield self.notifyChanged()
+        yield self.notifyPropertyChanged()
 
         queryCacher = self._home._txn._queryCacher
         if queryCacher:
@@ -3997,7 +3990,7 @@
                 self.viewerHome().uid(),
                 self._txn,
                 self._resourceID,
-                notifyCallback=self.notifyChanged
+                notifyCallback=self.notifyPropertyChanged
             )
         self.initPropertyStore(props)
         self._properties = props
@@ -4056,12 +4049,35 @@
         return self.ownerHome().notifierID()
 
 
-    @inlineCallbacks
     def notifyChanged(self):
         """
+        Send notifications when a child resource is changed.
+        """
+        return self._notifyChanged(property_change=False)
+
+
+    def notifyPropertyChanged(self):
+        """
+        Send notifications when properties on this object change.
+        """
+        return self._notifyChanged(property_change=True)
+
+
+    @inlineCallbacks
+    def _notifyChanged(self, property_change=False):
+        """
         Send notifications, change sync token and bump last modified because
         the resource has changed.  We ensure we only do this once per object
         per transaction.
+
+        Note that we distinguish between property changes and child resource changes. For property
+        changes we need to bump the collections revision token, but we must not do that for child
+        changes because that can cause a deadlock (plus it is not needed as the overall revision
+        token includes the child token via the max() construct in the query).
+
+        @param property_change: indicates whether this is the result of a property change as opposed to
+            a child resource being added, changed or removed.
+        @type property_change: C{bool}
         """
         if self._txn.isNotifiedAlready(self):
             returnValue(None)
@@ -4071,8 +4087,9 @@
         if self._resourceID:
             yield self.bumpModified()
 
-            # We now also bump the collection level sync token on any change
-            yield self._bumpSyncToken()
+            # Bump the collection level sync token only for property change
+            if property_change:
+                yield self._bumpSyncToken()
 
         # Send notifications
         if self._notifiers:
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.macosforge.org/pipermail/calendarserver-changes/attachments/20130625/d3c7c195/attachment-0001.html>


More information about the calendarserver-changes mailing list