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-1943 Implemented peer-to-peer indexing, based on processing of remote internal events. #827

Closed
wants to merge 1 commit into from

Conversation

hchiorean
Copy link
Member

This mechanism will be triggered whenever a repository is clustered and indexing is not configured in clustered mode.

…emote internal events. This mechanism will be triggered whenever a repository is clustered and indexing is not configured in clustered mode.
@@ -56,7 +55,7 @@ void addToIndex( String workspace,
Path path,
Name primaryType,
Set<Name> mixinTypes,
Collection<Property> properties,
Iterator<Property> propertiesIterator,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why you made this change. It does appear to simplify implementations very slightly (since they don't have to actually call iterator(), but since the supplied iterator can only be used once it means that it will no longer work if there are multiple monitors.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for the change is that the CachedNode interfaces expose node properties as iterators as opposed to collections. Therefore, when passing those properties to the monitor for indexing (in the case of remote events) I would've had to collect them in a collection - which would've been a performance issue (given the frequency of remote events in a cluster)

I do agree with the point about multiple monitors, but IMO given the way we use monitors atm, it doesn't seem likely that we'll have multiple monitors operating on the same set of properties. If sometime in the future this will be the case, we can easily add an overloaded method.

@rhauch
Copy link
Contributor

rhauch commented May 29, 2013

Everything looks good. I'll begin the merge.

@rhauch
Copy link
Contributor

rhauch commented May 29, 2013

Rebased and merged into the 'master' branch.

@rhauch rhauch closed this May 29, 2013
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