Uploaded image for project: 'Confluence Data Center'
  1. Confluence Data Center
  2. CONFSERVER-21645

Daily report stops functioning when user is removed directly from LDAP server

XMLWordPrintable

      Steps to reproduce issue
      1. Integrate Confluence with LDAP
      2. Ensure a local user and an LDAP user have Subscribed to daily updates
      3. Stop Confluence server
      4. Remove the LDAP user straight in LDAP server
      5. Start Confluence
      6. The daily report will fail with the following error: 
        2011-01-17 18:38:56,873 ERROR [DefaultQuartzScheduler_Worker-7] [org.quartz.core.ErrorLogger] schedulerError Job (DEFAULT.DailyReportJob threw an exception.
org.quartz.SchedulerException: Job threw an unhandled exception. [See nested exception: com.atlassian.core.exception.InfrastructureException: org.springframework.dao.InvalidDataAccessApiUsageException: Write operations are not allowed in read-only mode (FlushMode.NEVER): Turn your Session into FlushMode.AUTO or remove 'readOnly' marker from transaction definition.]
        at org.quartz.core.JobRunShell.run(JobRunShell.java:210)
        at org.quartz.simpl.SimpleThreadPool$WorkerThread.run(SimpleThreadPool.java:549)
Caused by: com.atlassian.core.exception.InfrastructureException: org.springframework.dao.InvalidDataAccessApiUsageException: Write operations are not allowed in read-only mode (FlushMode.NEVER): Turn your Session into FlushMode.AUTO or remove 'readOnly' marker from transaction definition.
        at com.atlassian.hibernate.HibernateObjectDao.remove(HibernateObjectDao.java:166)
        at com.atlassian.confluence.mail.notification.DefaultNotificationManager.removeNotification(DefaultNotificationManager.java:118)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
        at java.lang.reflect.Method.invoke(Method.java:597)
        at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:304)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:182)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:149)
        at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:106)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:171)
        at com.atlassian.spring.interceptors.SpringProfilingInterceptor.invoke(SpringProfilingInterceptor.java:16)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:171)
        at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:204)
        at $Proxy26.removeNotification(Unknown Source)
        at com.atlassian.confluence.mail.ChangeDigestNotificationBean.getAllChangeReports(ChangeDigestNotificationBean.java:176)
        at com.atlassian.confluence.mail.jobs.DefaultDailyReportManager.generateDailyReports(DefaultDailyReportManager.java:51)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
        at java.lang.reflect.Method.invoke(Method.java:597)
..
..
..
Caused by: org.springframework.dao.InvalidDataAccessApiUsageException: Write operations are not allowed in read-only mode (FlushMode.NEVER): Turn your Session into FlushMode.AUTO or remove 'readOnly' marker from transaction definition.
        at org.springframework.orm.hibernate.HibernateTemplate.checkWriteOperationAllowed(HibernateTemplate.java:1000)
        at org.springframework.orm.hibernate.HibernateTemplate$16.doInHibernate(HibernateTemplate.java:650)
        at org.springframework.orm.hibernate.HibernateTemplate.execute(HibernateTemplate.java:370)
        at org.springframework.orm.hibernate.HibernateTemplate.delete(HibernateTemplate.java:648)
        at org.springframework.orm.hibernate.HibernateTemplate.delete(HibernateTemplate.java:644)
        at com.atlassian.hibernate.HibernateObjectDao.remove(HibernateObjectDao.java:160)
        ... 38 more
~
      Findings

      In Confluence 2.10.3 and 3.3.1, this does not take place because of the following:

      2011-01-17 18:19:16,804 WARN [DefaultQuartzScheduler_Worker-6] [atlassian.confluence.mail.ChangeDigestNotificationBean] getAllChangeReports User Test not found for notification com.atlassian.confluence.mail.notification.Notification@110002 (removing notification)

      Workaround for 3.4.x

      1. Unzip CONF-21645-fix.zip into WEB-INF
      2. Make sure the following exists in your Confluence installation directory
        • confluence-install/confluence/WEB-INF/classes/com/atlassian/confluence/mail/ChangeDigestNotificationBean.class
      3. Restart Confluence

        1. CONF-21645-do-not-delete-notification.patch
          0.9 kB
          Don Willis
        2. CONF-21645-fix.zip
          4 kB
          Don Willis

            [CONFSERVER-21645] Daily report stops functioning when user is removed directly from LDAP server

            HuseinA added a comment -

            putting the workaround in the description

            HuseinA added a comment - putting the workaround in the description

            Tom Bell added a comment -

            Don,
            Good workaround! Our daily reports are working again and users are happy.
            Thank you!!!!

            Tom Bell added a comment - Don, Good workaround! Our daily reports are working again and users are happy. Thank you!!!!

            Don Willis added a comment - - edited

            Workaround for 3.4.x

            1. Unzip CONF-21645-fix.zip into WEB-INF
            2. Restart Confluence

            Don Willis added a comment - - edited Workaround for 3.4.x Unzip CONF-21645-fix.zip into WEB-INF Restart Confluence

            Don Willis added a comment -

            Implemented by removing the attempt to remove notification records during the notification job. It's not worth the performance hit. If this was important functionality we could add a background job to do the cleanup, but I think it was only written as a convenience.

            Don Willis added a comment - Implemented by removing the attempt to remove notification records during the notification job. It's not worth the performance hit. If this was important functionality we could add a background job to do the cleanup, but I think it was only written as a convenience.

            Don Willis added a comment -

            I recreated Craig's tests from http://jira.atlassian.com/browse/CONF-13875?focusedCommentId=209501&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-209501 which completed in reasonable time on my machine with no changes, and took hours when I removed the readOnly. So it wasn't just about being in a single transaction and was very much because it's a readonly transaction. (or something went wrong with my test)

            The removal definitely seems like more effort than it is currently worth. It would take a lot of looking up spurious notification records to be equivalent to the slowdown of having a read/write transaction for this operation.

            Don Willis added a comment - I recreated Craig's tests from http://jira.atlassian.com/browse/CONF-13875?focusedCommentId=209501&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-209501 which completed in reasonable time on my machine with no changes, and took hours when I removed the readOnly. So it wasn't just about being in a single transaction and was very much because it's a readonly transaction. (or something went wrong with my test) The removal definitely seems like more effort than it is currently worth. It would take a lot of looking up spurious notification records to be equivalent to the slowdown of having a read/write transaction for this operation.

            Don Willis added a comment - - edited

            From Craig P

            The change was instead of 1 transaction per notification, there's 1 transaction for the entire job.

            + User details, etc get cached across notifications
            + Performance increase significant (e.g. 6 mins vs 12 hours for large systems will lots of users / changes)

            • One long running transaction (i.e. read locks, and potentially write locks kept - depends on DB flavour).

            Simple fix is to remove the readOnly, and it should sort itself out. The job is pretty quick in most cases, so shouldn't keep locks too long.

            Other fixes: do writes in a separate txn at the end (i.e. after the read only txn is closed to avoid potential deadlocks); or break down the job into batches (e.g. 100 notifications per txn) to balance performance vs txn length.

            Don Willis added a comment - - edited From Craig P The change was instead of 1 transaction per notification, there's 1 transaction for the entire job. + User details, etc get cached across notifications + Performance increase significant (e.g. 6 mins vs 12 hours for large systems will lots of users / changes) One long running transaction (i.e. read locks, and potentially write locks kept - depends on DB flavour). Simple fix is to remove the readOnly, and it should sort itself out. The job is pretty quick in most cases, so shouldn't keep locks too long. Other fixes: do writes in a separate txn at the end (i.e. after the read only txn is closed to avoid potential deadlocks); or break down the job into batches (e.g. 100 notifications per txn) to balance performance vs txn length.

            Don Willis added a comment -

            I've had a quick look at this, and I believe the reason for the bug is that a transaction proxy was effectively wrapped around the ChangeNotificationDigestBean:

                <bean id="dailyReportManager" class="org.springframework.transaction.interceptor.TransactionProxyFactoryBean" plugin:available="true">
        <property name="transactionManager" ref="transactionManager"/>
        <property name="target">
            <ref local="dailyReportManagerTarget"/>
        </property>
        <property name="transactionAttributes">
            <props>
                <prop key="*">PROPAGATION_REQUIRED,readOnly</prop>
            </props>
        </property>
        <property name="proxyInterfaces">
            <value>com.atlassian.confluence.mail.jobs.DailyReportManager</value>
        </property>
        <property name="postInterceptors">
            <list>
                <ref local="profilingInterceptor"/>
            </list>
        </property>
    </bean>

            If we take out that "readonly" then the bug should go away (I think).
            However, I guess that readonly was added for valid performance reasons which we probably don't want to revert. What we probably should do is stop removing the notification while processing them instead. That removal is probably a bad idea anyway, since it means if the LDAP repository is temporarily unreachable then the user's notifications will be removed.

            Don Willis added a comment - I've had a quick look at this, and I believe the reason for the bug is that a transaction proxy was effectively wrapped around the ChangeNotificationDigestBean: <bean id= "dailyReportManager" class= "org.springframework.transaction.interceptor.TransactionProxyFactoryBean" plugin:available= "true" > <property name= "transactionManager" ref= "transactionManager" /> <property name= "target" > <ref local= "dailyReportManagerTarget" /> </property> <property name= "transactionAttributes" > <props> <prop key= "*" > PROPAGATION_REQUIRED,readOnly </prop> </props> </property> <property name= "proxyInterfaces" > <value> com.atlassian.confluence.mail.jobs.DailyReportManager </value> </property> <property name= "postInterceptors" > <list> <ref local= "profilingInterceptor" /> </list> </property> </bean> If we take out that "readonly" then the bug should go away (I think). However, I guess that readonly was added for valid performance reasons which we probably don't want to revert. What we probably should do is stop removing the notification while processing them instead. That removal is probably a bad idea anyway, since it means if the LDAP repository is temporarily unreachable then the user's notifications will be removed.

              don.willis@atlassian.com Don Willis
              sjayaraman Sashidaran Jayaraman [Atlassian]
              Affected customers:
              7 This affects my team
              Watchers:
              9 Start watching this issue

                Created:
                Updated:
                Resolved: