Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MODE-2301 Added support for indexes to be updated synchronously (the default) or asynchronously #1245

Merged
merged 3 commits into from Sep 12, 2014

Conversation

rhauch
Copy link
Contributor

@rhauch rhauch commented Sep 11, 2014

Prior to this commit, all updates to the indexes in the 4.0 codebase were asynchronous. That means that the Session.save() method added the ChangeSet describing the session's changes onto the event bus and then returned. All index updates were done in separate listener threads on the event bus, so it's entirely possible (and perhaps likely) that any updates to the indexes happen after the client's call to Session.save() returned. The net effect is that the client might make changes and immediately issue a query that would not find the recently-saved changes. Additionally, there was no way for the client to know when the indexes would be updated.

With this commit, it is possible to define whether each index is updated synchronously before the Session.save() returns, or asynchronously so that index updates are made in a separate thread. This commit changes the default behavior of the index updates to be synchronous.

In a cluster, any changes made on one process are sent via the change bus to the other processes, and the indexes in those other processes are always updated asynchronously. In other words, if an index provider keeps copies of the indexes on every process, then only the local indexes are updated synchronously before save returns -- and that Session.save() does not wait until the indexes in the other processes are updated.

In order to properly implement this, a small change was made to the ChangeBus to expand the semantic concept of an "in-thread" listener. Prior to this, "in-thread" meant that the listener only received locally-originating change sets. Now, it's still possible to do this (and the journal uses this older behavior), but it's also possible to register a listener such that locally-originating change sets are handled in-thread while remotely originating change sets are handled asynchronously.

…d APIs from the recently-updated Apache HttpClient library.
@rhauch
Copy link
Contributor Author

rhauch commented Sep 11, 2014

@hchiorean, please take a critical look at the changes to the ChangeBus interface and the RepositoryChangeBus and ClusteredChangeBus implementations. I want to make sure that the journal (which is still added via bus.registerInThread(journal)) is being processed correctly, and that the local journal should not receive remotely-originating events. BTW, this is why registerInThread(observer) calls registerInThread(observer,false).

I needed the synchronous indexes to be registered using registerInThread(index,true) so that remotely-originating events are still sent to the index using the normal asynchronous bus listener mechanism, but that locally-originating events are added to the bus as normal but also handled by the in-thread listeners.

@hchiorean
Copy link
Member

@rhauch: as commented above, I don't really understand the change to the ChangeBus interface and I don't think this is needed (not to mention it makes a complicated part of the code even more complicated, which is not good IMO)

For the index provider(s) to be notified "in thread" it should suffice to register them via the regular registerInThread method. All remote events will be processed off another thread regardless (again, via JGroups)

…ynchronously

Prior to this commit, all updates to the indexes in the 4.0 codebase were asynchronous. That means that
the Session.save() method added the ChangeSet describing the session's changes onto the event bus and
then returned. All index updates were done in separate listener threads on the event bus, so it's entirely
possible (and perhaps likely) that any updates to the indexes happen after the client's call to Session.save()
returned. The net effect is that the client might make changes and immediately issue a query that would not
find the recently-saved changes. Additionally, there was no way for the client to know when the indexes would
be updated.

With this commit, it is possible to define whether each index is updated synchronously before the
the Session.save() returns, or asynchronously so that index updates are made in a separate thread.
This commit changes the default behavior of the index updates to be *synchronous*.

In a cluster, any changes made on one process are sent via the change bus to the other processes,
and the indexes in those other processes are always updated asynchronously. In other words, if an
index provider keeps copies of the indexes on every process, then only the local indexes are updated
synchronously before save returns -- and that Session.save() does not wait until the indexes in the other
processes are updated.

In order to properly implement this, a small change was made to the ChangeBus to expand the semantic concept
of an "in-thread" listener. Prior to this, "in-thread" meant that the listener only received locally-originating
change sets. Now, it's still possible to do this (and the journal uses this older behavior), but it's also
possible to register a listener such that locally-originating change sets are handled in-thread while remotely
originating change sets are handled asynchronously.
@rhauch
Copy link
Contributor Author

rhauch commented Sep 12, 2014

Thanks for the feedback, @hchiorean. I now understand how the ChangeBus and implementations work, and you are correct that I don't need to modify those classes. I've changed my second commit to remove those changes (along with the changes for a few other classes). The result is something that is very focused on changes to only index-related code and index definitions (including JSON repository configurations the JBoss subsystem).

This should be ready for merge.

@hchiorean
Copy link
Member

@rhauch this looks good. The only thing missing IMO (sorry for not catching it earlier) is a WF integration test of the new attribute.

rhauch added a commit that referenced this pull request Sep 12, 2014
MODE-2301 Added support for indexes to be updated synchronously (the default) or asynchronously
@rhauch rhauch merged commit e618b5b into ModeShape:master Sep 12, 2014
@rhauch rhauch deleted the mode-2301 branch September 23, 2014 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants