Uploaded image for project: 'Sling'
  1. Sling
  2. SLING-3747

Provide a way to signal Jcr Installer to pause and resume scanning

    Details

    • Type: New Feature
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: JCR Installer 3.1.8
    • Component/s: Installer
    • Labels:
      None

      Description

      Currently Sling Installer JCR Provider would listen for observation event and would perform a rescan every 500 msec upon receiving any observation event.

      This at times cause issue when large number of bundles get updated in repository say via installation of bug fixing content package. Further if same content package also updates the repository bundle then it causes issues as repository bundle might get updated midway while content package is still being deployed

      Based on Felix Meschberger suggestion this can be done via introducing a signalling mechanism between component which installs package and JCR Installer. It would work something like this

      1. JCR Installer would check if there is any node under /system/sling/installer/jcr/pauseInstallation as a signal to pause installation.
      2. Package installer would create a child node under the above path before installing the package and remove it post installation

      This should also address the issue in cluster deployment

      1. SLING-3747.patch
        13 kB
        Chetan Mehrotra
      2. SLING-3747-1.patch
        14 kB
        Chetan Mehrotra
      3. SLING-3747-2.patch
        9 kB
        Chetan Mehrotra

        Issue Links

          Activity

          Hide
          chetanm Chetan Mehrotra added a comment -

          Initial patch which implements above approach.

          Pending stuff

          • Support for some kind of timeout.
          Show
          chetanm Chetan Mehrotra added a comment - Initial patch which implements above approach. Pending stuff Support for some kind of timeout.
          Hide
          fmeschbe Felix Meschberger added a comment -

          Thanks for providing the patch.

          Some remarks:

          • runOneCycle should probably also check for the property
          • I have the suspicion, that the property event may not always be caught if the /system/sling/installer/jcr node is removed instead of the property. In fact the listener is also not triggered if the property is removed.

          I wonder whether it would be sufficient to just check for the property during runOneCycle and ignore the cycle if the property is set ?

          Show
          fmeschbe Felix Meschberger added a comment - Thanks for providing the patch. Some remarks: runOneCycle should probably also check for the property I have the suspicion, that the property event may not always be caught if the /system/sling/installer/jcr node is removed instead of the property. In fact the listener is also not triggered if the property is removed. I wonder whether it would be sufficient to just check for the property during runOneCycle and ignore the cycle if the property is set ?
          Hide
          chetanm Chetan Mehrotra added a comment -

          Updated patchSLING-3747-1.patch

          I have the suspicion, that the property event may not always be caught if the /system/sling/installer/jcr node is removed instead of the property. In fact the listener is also not triggered if the property is removed.

          Updated the code to also look for property removed. It would address both the above cases

          I wonder whether it would be sufficient to just check for the property during runOneCycle and ignore the cycle if the property is set ?

          The check is being done in the while loop which invokes the runOneCycle. If we trip inside runOneCycle then it might cause issue as needScan flag of various folders would have been reset and we might loose info from those folders if no change happen in them by the time we run next cycle while running

          Show
          chetanm Chetan Mehrotra added a comment - Updated patch SLING-3747-1.patch I have the suspicion, that the property event may not always be caught if the /system/sling/installer/jcr node is removed instead of the property. In fact the listener is also not triggered if the property is removed. Updated the code to also look for property removed. It would address both the above cases I wonder whether it would be sufficient to just check for the property during runOneCycle and ignore the cycle if the property is set ? The check is being done in the while loop which invokes the runOneCycle. If we trip inside runOneCycle then it might cause issue as needScan flag of various folders would have been reset and we might loose info from those folders if no change happen in them by the time we run next cycle while running
          Hide
          cziegeler Carsten Ziegeler added a comment -

          What about - as Felix suggest - simply check the value of the property before runOneCycle() is invoked instead of relying on observation to be deliverd in the correct order?
          I haven't looked at the code in detail, but maybe a session.refresh() needs to be added as well.

          Show
          cziegeler Carsten Ziegeler added a comment - What about - as Felix suggest - simply check the value of the property before runOneCycle() is invoked instead of relying on observation to be deliverd in the correct order? I haven't looked at the code in detail, but maybe a session.refresh() needs to be added as well.
          Hide
          chetanm Chetan Mehrotra added a comment -

          Updated patch which does not rely on observation. Instead a check for property value is made if any of the watched folder needs scan as per Felix Meschberger suggestion

          What about - as Felix suggest - simply check the value of the property before runOneCycle() is invoked instead of relying on observation to be deliverd in the correct order?

          Well that becomes polling and making JCR call every 500 msec. Might be better to check for property value only if any of the WatchedFolder.needsScan returns true. Then we would not be making too frequent JCR calls.

          Updated patch to use that approach

          but maybe a session.refresh() needs to be added as well.

          When session with which ObservationListener is registerd is used to read a value post event delivery then as per spec its ensured that session is upto date.

          Show
          chetanm Chetan Mehrotra added a comment - Updated patch which does not rely on observation. Instead a check for property value is made if any of the watched folder needs scan as per Felix Meschberger suggestion What about - as Felix suggest - simply check the value of the property before runOneCycle() is invoked instead of relying on observation to be deliverd in the correct order? Well that becomes polling and making JCR call every 500 msec. Might be better to check for property value only if any of the WatchedFolder.needsScan returns true. Then we would not be making too frequent JCR calls. Updated patch to use that approach but maybe a session.refresh() needs to be added as well. When session with which ObservationListener is registerd is used to read a value post event delivery then as per spec its ensured that session is upto date.
          Hide
          cziegeler Carsten Ziegeler added a comment -

          Isn't there an observation event triggering the installer to do work? So we can skip the check, if nothing is to do

          Show
          cziegeler Carsten Ziegeler added a comment - Isn't there an observation event triggering the installer to do work? So we can skip the check, if nothing is to do
          Hide
          chetanm Chetan Mehrotra added a comment -

          Isn't there an observation event triggering the installer to do work? So we can skip the check, if nothing is to do

          Yes the new [patch|^SLING-3737-2.patch] takes that approach only , check for property value is made only if any of the watched folder needs scan

          Show
          chetanm Chetan Mehrotra added a comment - Isn't there an observation event triggering the installer to do work? So we can skip the check, if nothing is to do Yes the new [patch|^SLING-3737-2.patch] takes that approach only , check for property value is made only if any of the watched folder needs scan
          Hide
          cziegeler Carsten Ziegeler added a comment -

          Lgtm, thanks

          Show
          cziegeler Carsten Ziegeler added a comment - Lgtm, thanks
          Hide
          bdelacretaz Bertrand Delacretaz added a comment -

          IIUC /system/sling/installer/jcr/pauseInstallation only allows one client to pause the installer - if two clients use this mechanism (adding/removing the property around installation operations), they might step on each other's toes.

          It would be cleaner IMO to make /system/sling/installer/jcr/pauseInstallation a node, require clients to create child nodes with unique names like com.example.packages.someInstaller, and pause the installer as long as at least one node exists under /system/sling/installer/jcr/pauseInstallation

          Show
          bdelacretaz Bertrand Delacretaz added a comment - IIUC /system/sling/installer/jcr/pauseInstallation only allows one client to pause the installer - if two clients use this mechanism (adding/removing the property around installation operations), they might step on each other's toes. It would be cleaner IMO to make /system/sling/installer/jcr/pauseInstallation a node, require clients to create child nodes with unique names like com.example.packages.someInstaller, and pause the installer as long as at least one node exists under /system/sling/installer/jcr/pauseInstallation
          Hide
          chetanm Chetan Mehrotra added a comment -

          It would be cleaner IMO to make /system/sling/installer/jcr/pauseInstallation a node, require clients to create child nodes with unique names like com.example.packages.someInstaller, and pause the installer as long as at least one node exists under /system/sling/installer/jcr/pauseInstallation

          Good point. Would go for that approach

          Show
          chetanm Chetan Mehrotra added a comment - It would be cleaner IMO to make /system/sling/installer/jcr/pauseInstallation a node, require clients to create child nodes with unique names like com.example.packages.someInstaller, and pause the installer as long as at least one node exists under /system/sling/installer/jcr/pauseInstallation Good point. Would go for that approach
          Hide
          chetanm Chetan Mehrotra added a comment -

          Implemented the above logic in http://svn.apache.org/r1617254

          The logic would check for presence of any child node in path /system/sling/installer/jcr/pauseInstallation (default value) as an indicator that JCR integration should be paused. If no child is found the normal processing would be done

          Show
          chetanm Chetan Mehrotra added a comment - Implemented the above logic in http://svn.apache.org/r1617254 The logic would check for presence of any child node in path /system/sling/installer/jcr/pauseInstallation (default value) as an indicator that JCR integration should be paused. If no child is found the normal processing would be done

            People

            • Assignee:
              chetanm Chetan Mehrotra
              Reporter:
              chetanm Chetan Mehrotra
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development