enhancement: Ensure Oak Index feature should respond to changes in the defined ensure oak index 'config' nodes

[adamyocum]

What I am seeing is that when I update and deploy changes to my /apps/myapp/oak-index/* files the changes don't get evaluated by the Ensure feature until i stop and start the ACS commons bundle.

Re-installing the config doesn't seem to cause a re-registering / activating either of Ensure, even deploying acs again won't restart the Ensure service since there are no version changes to the bundle.

I brought this up in the ACS chat and there was some discussion. It seems this is an area that needs development, the idea I like so far is creating a listener and the ensure service watches for changes in any of its defined index config locations, then Ensure re-scans the indexes for changes.

[joerghoh]

Yes, the Ensure Oak Index feature lacks support for runtime changes to the index definitions. When I reworked the initial implementation, I wasn't thinking of this requirement, because on runtime we don't do changes, but rather during deployments. But even on deployment I cannot say for sure, that first the new index definitions are deployed and then the Ensure Oak Index component gets started.

Adding support to detect changes to the index definitions might get a bit more complex, because I don't know for sure the behaviour of Oak, when I change an index which is currently reindexing. Also in case of Oak property indexes the change to the index in /oak:index/myOakPropertyIndex is not visible until the reindexing has finished. Starting the rescan (while a reindexing is already running) will try to change the index again; this is also a case where I don't know the expected or actual behaviour of Oak.

I try to come up with some ideas.

[adamyocum]

thanks @joerghoh I will keep thinking too.. over the holiday break maybe we can let the ideas settle some... I would be ok with just a hook we could hit that would re-load the component (or at least re-scan the index defs)... Something like a rest endpoint trigger could easily be added to our maven job. Also thinking more on that listener approach I liked it at first, but I could see problems like what if someone starts manually editing the configs in crx/de, it would be firing like crazy, so we would need to handle that case gracefully which could just get messy it seems... hmm ok well it is the end of the day here, but let's continue this later!

[davidjgonzalez]

@hsaginor this is the ensure index thread

Can you make a PR from your branch https://github.com/hsaginor/acs-aem-commons/tree/ensure-listener/bundle/src/main/java/com/adobe/acs/commons/oak/impl to acs master? Makes seeing your changes easier and facilitates a convo via comments. Worst case we decline the PR.

[joerghoh]

The current approach tries to do everything automatically, but I don't think, that this the best idea. Especially when your deployment process is completely under your control, you should be able to apply all accumulated changes based on your own schedule. So you should have the possibility to disable all automatic triggers and start the EnsureIndex functionality via a http call or via code.

[hsaginor]

Hi @joerghoh Thank you for your input. We do want the actual deployment of indexes to be automated somehow (as most projects would probably). The actual mechanics of it could be a listener or an HTTP call initiated by the build system or other code based trigger as you suggested. In our case ensure definitions would change only from a build and only on our schedule. But I do see how given your concern regarding reindexing this could be an issue in some cases. For example if you decide to build some fancy UI to manage Ensure definitions or if someone decides to change properties in CRXDE lite.

Perhaps automated updates can be optional/configurable. I could also implement some HTTP interface to trigger it if you think it would be useful. I guess we could always restart the OSGi bundle. Another idea is to control automated updates via a special property. So, only if this property is set to true update the indexes.

BTW do you know if there is anyway to programmatically detect if reindexing is running?

[joerghoh]

I comitted an idea how it can work on https://github.com/joerghoh/acs-aem-commons/tree/feature/607-oak-ensure-index

Basically it adds the feature, that you can "postpone" the application of the index definition to the system (just add the property "oak-index.applyOnStartup = false" to the OSGI config of the respective EnsureOakDefinition). Then you either use the service "OakIndexManager" to apply all pending indexing definitions to the system. Or you can use the webconsole (/system/console/EnsureOakIndex) to start this process. The default behaviour stays the same, this is an optional feature.

@hsaginor Does this satisfy your needs? You can easily use curl to call the webconsole or implement your own servlet.

@adamyocum I've thought a bit on the requirement to start the application of the definition automatically. Personally I am not a fan of it, as too many things can go wrong (just think of insufficient definitions and long Oak reindexing times); I built this as a tool for a production environment (or any non-local-developer environment), and there such changes should be very rare, so the automatic application doesn't bring real benefit. Regarding my code posted above: At the moment I there's explicit code in EnsureOakIndex, which avoids to apply the same definition multiple times (and ignoring any change on the definitions itself). We probably need to change some bits to speedup the roundtrip of

• change index definitions
• apply the index definition
• test the outcome

But maybe this approach is wrong in itself, and you should rather use

• change the Oak index directly
• test the outcome

and when the outcome is good, migrate the index definition into the format EnsureOakIndex expects. WDYT?

[davidjgonzalez]

@joerghoh,

WDYT about decoupling this into..

• EnsureIndex = responsible for ensuring itself; agnostic to when
• EnsureIndexHandlers = responsible for telling when an EnsureIndex should be applied
• OakIndexManager = responsible for coordinating Handler and Indexes; Can also implement a Servlet and/or MBean interface to apply named Indexes (ensure definition path is a good "id")

Then folks could config the ensureIndexes to be whatever they want. If someone has a usecase to do it onchange like @hsaginor, they can specify that handler key in their EnsureIndex osgi config for config.dev. If later on you want to create a scheduled job (the next Friday at 11pm), you could create a new handler for that.

Ensure Index
@Property path
@Property handlers = [ activate, onchange ]

EnsureIndexHandler
@handles = [ activate ]

void handle(EnsureIndex index)
void unhandle(EnsureIndex index)

The Handlers required to meet the requested functionality is

• ImmediateHandlerImpl ~> ~ calls the code currently in EnsureIndex.activate(..)
• OnChangeHandlerImpl ~> ~ @hsaginor 's EventListener code

OakIndexManagerImpl implements OakIndexManager, HttpServlet
@Component immediate=true
@Reference ensureIndexes as this.indexes
@Reference ensureIndexHandlers as this.handlers

// Not sure if service registration could cause timing issues w this, thus the handling via each vector.
// Handlers would need to ensure duplicate work isnt being done, and noop .. or OakIndexManager could track the processing of a handler/index combo itself.

bindEnsureIndex(ensureIndex, config) {
  for(EnsureIndexHandlers handler : this.handlers) {
    handler.handle(ensureIndex);
  }
}

unbindEnsureIndex(ensureIndex, config) {
  for(EnsureIndexHandlers handler : this.handlers) {
    handler.unhandle(ensureIndex);
  }
}

bindEnsureIndexHandler(handler, config) {
  for(EnsureIndexes ensureIndex : this.indexes) {
    handler.unhandle(ensureIndex);
  }
}

unbindEnsureIndexHandler(handler, config) {
  for(EnsureIndexes ensureIndex : this.indexes) {
    handler.unhandle(ensureIndex);
  }
}

[joerghoh]

@davidjgonzalez I want to keep this whole thing as simple as possible. Adding interfaces here and there makes it far too complex.

The usecases I want to support and which I need:

• Just use EnsureOakIndex to bring in new/updated indexes and let the tool to everything else
• Use EnsureOakIndex and have a way to apply the definitions when you need it, which is why I added the OakIndexManager and the Webconsole interface

Everything else (like scheduled application etc) should be done outside of ACS Commons, because then it's getting (too) complex and specific. Even my comment is a bit speculative, as I am not sure if I need it at all.

So I really appreciate your solution as a nice and elegant one, but it introduces extension points which we are IMHO unlikely to need.

[adamyocum]

@joerghoh the webconsole interface and new OakIndexManager service additions will support all my usecases needed and I agree that a listener is too complicated and not something I would want after thinking about it. Thanks for exploring the options and I think we have a good solution here IMHO.

[hsaginor]

@joerghoh Thanks for taking the time to explain the details. After giving it more thought I do agree that adding a listener to ACS commons may not be the best idea. It would probably serve our purpose. But that was a bit short sighted on my part.

I did do another push to https://github.com/hsaginor/acs-aem-commons/tree/ensure-listener that implemented a conditional similar to your "oak-index.applyOnStartup = false" feature. It also added the same configuration option for applying changes via listener (set to false by default). But that was before you posted your update. I was going to implement something similar to your OakIndexManager and the webconsole but just did not get to it over the holidays. I like this approach.

Please let me know what you think of the following idea. Can we add a "revision" property to EnsureOakIndex where the value basically indicates the revision of ensure definitions. That can easily be incremented by a maven build when we want to apply changes and cause the EnsureOakIndex service to restart without any listeners or http calls to the web console. WDYT?

[joerghoh]

@hsaginor What requirement do you want to solve with this "revision" property?

I forgot to mention that this "avoid to apply the same definition multiple times" only works if you don't restart the service/bundle/AEM instance. If you restart the service, it is tried to apply the index definition again. Which (of course) is only executed if the EnsureOakIndex definition has changed compared to the real Oak index. So when you deploy a new EnsureOakIndex definition and you restart the service/bundle/instance it should work without any further work.

[hsaginor]

@joerghoh The requirement is to automatically apply changes to our custom oak indexes when we deploy them. The problem with original implementation of EnsureOakIndex is that definitions are copied to /oak-index only on service activation. That's what @adamyocum described when he opened this ticket. A change to service configuration via a "revision" property would cause the service instance to restart. And the revision property can be updated by the build only when definitions actually change (pulled from version control system for example).

Note that since EnsureOakIndex is a factory only an instance (if we have multiple) that has changed definitions would change and re-apply. And this is as simple as it gets to implement - just add an SCR property that does not need to be evaluated in code. This would be an optional property of course.

I do see how this might have the same issues you stated before. For example if ensure definitions are deployed and applied during long re-indexing. But we control when we run our builds and I don't see a clear way to make sure this does not happen anyway. Someone could even apply definitions at the wrong time via your web console plugin. The plugin provides a nice interface but does not avoid the problem. And if this causes any unwanted side effect I see it as potentially a bug in oak since it should be able to handle any index updates more gracefully, even delaying actual indexing on it's own.

[joerghoh]

@hsaginor So you are just looking for a way to automatically trigger a restart of the service instance, and you would "force" this restart by a changed "revision" property when you deploy new EnsureOakIndex definitions?

I think, that this already works. When you update a property on a sling:OsgiConfig node, the service bound to this config is restarted. And neither the JCRInstaller nor SCR care about if your service is using this property.

[hsaginor]

Oh yes. SCR does not care if the service is using it. Just setting a property on sling:OsgiConfig node works.

[davidjgonzalez]

@joerghoh did you have a post somewhere about securing the HTTP end-point? I recall seeing something somewhere but cant find it now.

[davidjgonzalez]

We can re-open, but closing due to lack of interesting and this also auto-reindexing can be pretty dangerous.

Also some work around are provided w/ the webconsole end point and JMX mbeans