Fixes related to mutl-domain deployments #238

abmargb · 2014-09-27T14:50:50Z

Now the external-domain-checker returns a list of local domains that can be reused between calls of ChannelManager.isLocalDomain(), ChannelManager.isLocalJid() or ChannelManager.isLocalNode().
It drastically improves the perfomance of disco#items, purging remote cache, etc,... and allows for a selective query of local nodes.

Since HSQLDB is not fully compliant with postgres, I had to Mockito.spy the SQL connection object in the tests in order to replace ~ with regexp_matches.

#236

lloydwatkin · 2014-09-28T17:47:35Z

Can you merge in master please ?

lloydwatkin · 2014-09-28T17:49:38Z

src/main/java/org/buddycloud/channelserver/channel/ChannelManagerImpl.java

 		if (false == nodeId.matches("/user/.+@.+/.+")) {
 			logger.debug("Node " + nodeId + " has an invalid format");
 			throw new IllegalArgumentException(INVALID_NODE);
 		}
 		String domain = new JID(nodeId.split("/")[2]).getDomain();
-		return isLocalDomain(domain);
+		return isLocalDomain(domain, localDomains);


Why are we passing this value around as an argument when it can just be called internally?

It helps solving the performance issues we currently have. In cases we need to check if a lot of nodes are local - for example, in a for loop - we just need to execute the local-domain-checker once, and then pass this list around.

Where are we doing checks like this? I'm sure there's a better solution for this.

https://github.com/buddycloud/buddycloud-server-java/blob/multi-domain-fixes/src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/discoitems/DiscoItemsGet.java#L78.

The alternate solution would be to create a areLocalDomains() method returning a List of objects (or a map) that associates domains to the information stating they are local or not.

If we don't provide anything similar we simply lose the performance benefits of returning a csv in the local-domain-checker.

Why not pass the local domains through to the database query for retrieving local domains? (At most you'll have a standard and topics domain, but even that can be looped). That way it'll be quick in the DB and the code will be less complex.

That's exaclty what I did in the first place, but I thought we wanted to check it in a more explicit way, i.e. in the PacketProcessor itself.

Good. So I'll create a ChannelManager.getLocalNodeList() method that will make this simpler.

Conflicts: src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/items/special/FirehoseGet.java

@lloydwatkin

enhancements as suggested by @lloydwatkin.

lloydwatkin · 2014-10-03T10:26:31Z

src/main/java/org/buddycloud/channelserver/channel/ChannelManagerImpl.java

@@ -250,6 +250,11 @@ public void createPersonalChannel(JID owner) throws NodeStoreException {
 	}

 	@Override
+	public boolean isLocalDomain(String domain) {
+		return LocalDomainChecker.isLocal(domain, configuration);


Would it be better to get the local domain data via the Configuration object? This reduces the dependencies for the channel manager and means we are always getting 'configuration' data from the same place?

Hm, good point. I'm changing this.

lloydwatkin · 2014-10-03T10:35:54Z

A few comments, one potentially important.

Tests failed but I think that was due to an internet hiccup rather than your code. Have restarted the travisci run.

abmargb · 2014-10-04T15:43:15Z

@lloydwatkin Indeed it was. Thanks for the comments.

class.

abmargb · 2014-10-06T17:48:23Z

I have removed the Configuration dependency from the ChannelManager. This change had a huge impact in the tests - that's why it took me a while to get all tests fixed.

I've also noticed sometimes we refer the configuration object as non-singleton. I'd rather not use the Configuration as a singleton anywhere, since it makes tests more unpredictable. But let's just try to follow one style or another. I've raised an issue about this: #241.

Conflicts: src/main/java/org/buddycloud/channelserver/ChannelsEngine.java src/main/java/org/buddycloud/channelserver/Configuration.java src/main/java/org/buddycloud/channelserver/channel/ChannelManager.java src/main/java/org/buddycloud/channelserver/channel/ChannelManagerFactoryImpl.java src/main/java/org/buddycloud/channelserver/channel/ChannelManagerImpl.java src/main/java/org/buddycloud/channelserver/channel/LocalDomainChecker.java src/main/java/org/buddycloud/channelserver/db/NodeStore.java src/main/java/org/buddycloud/channelserver/db/jdbc/JDBCNodeStore.java src/main/java/org/buddycloud/channelserver/db/jdbc/dialect/Sql92NodeStoreDialect.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/discoinfo/DiscoInfoGet.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/discoitems/DiscoItemsGet.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/mam/MessageArchiveManagement.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/AffiliationsGet.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/NodeConfigureGet.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/NodeThreadsGet.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/RecentItemsGet.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/RepliesGet.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/SubscriptionsGet.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/ThreadGet.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/UserItemsGet.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/items/NodeItemsGet.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/items/SpecialItemsGet.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/items/special/FirehoseGet.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/result/ItemsResult.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/result/SubscriptionsResult.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/AffiliationEvent.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/ItemDelete.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/NodeConfigure.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/NodeCreate.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/NodeDelete.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/SubscribeSet.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/SubscriptionEvent.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/UnsubscribeSet.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/register/RegisterSet.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/register/UnregisterSet.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/search/Search.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/search/SearchGet.java src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/search/SearchSet.java src/main/java/org/buddycloud/channelserver/packetprocessor/message/event/AbstractMessageProcessor.java src/main/java/org/buddycloud/channelserver/packetprocessor/message/event/AffiliationProcessor.java src/main/java/org/buddycloud/channelserver/packetprocessor/message/event/ConfigurationProcessor.java src/main/java/org/buddycloud/channelserver/packetprocessor/message/event/DeleteProcessor.java src/main/java/org/buddycloud/channelserver/packetprocessor/message/event/ItemsProcessor.java src/main/java/org/buddycloud/channelserver/packetprocessor/message/event/RetractItemProcessor.java src/main/java/org/buddycloud/channelserver/packetprocessor/message/event/SubscriptionProcessor.java src/test/java/org/buddycloud/channelserver/channel/ChannelManagerImplTest.java src/test/java/org/buddycloud/channelserver/channel/TestHelper.java src/test/java/org/buddycloud/channelserver/channel/validate/AtomEntryTest.java src/test/java/org/buddycloud/channelserver/db/jdbc/DatabaseTester.java src/test/java/org/buddycloud/channelserver/db/jdbc/JDBCNodeStoreAbstract.java src/test/java/org/buddycloud/channelserver/db/jdbc/JDBCNodeStoreTest.java src/test/java/org/buddycloud/channelserver/packetHandler/iq/TestHandler.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/discoitems/DiscoItemsGetTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/mam/MessageArchiveManagementTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/NodeConfigureGetTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/NodeThreadsGetTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/SubscriptionsGetTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/items/NodeItemsGetTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/items/UserSingleItemGetTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/items/special/FirehoseGetTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/result/ItemsResultTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/AffiliationEventTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/ItemDeleteTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/NodeConfigureTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/NodeCreateTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/NodeDeleteTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/PublishTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/SubscribeSetTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/SubscriptionEventTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/UnsubscribeSetTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/register/RegisterSetTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/register/UnregisterSetTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/search/SearchGetTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/search/SearchSetTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/message/event/AffiliationProcessorTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/message/event/ConfigurationProcessorTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/message/event/DeleteProcessorTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/message/event/ItemsProcessorTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/message/event/NotificationSendingMockTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/message/event/RetractItemProcessorTest.java src/test/java/org/buddycloud/channelserver/packetprocessor/message/event/SubscriptionProcessorTest.java

coveralls · 2014-10-09T20:15:19Z

Coverage increased (+0.22%) when pulling bc01763 on multi-domain-fixes into bdb00aa on master.

Fixes related to mutl-domain deployments

coveralls · 2014-10-10T02:29:26Z

Coverage increased (+0.08%) when pulling 102761c on multi-domain-fixes into bdb00aa on master.

abmargb added 14 commits September 19, 2014 11:42

Removing unused variables and useless imports.

0434fb2

Removing unused fields from tests

542c8ff

Fixing style (missing brackets, weird one-liners...)

a0141de

more missing braces

123ed47

Fixes #235

80968eb

Returns every node from every domain in /firehose. Refers to #234.

f717b61

firehose: fixing pagination with rsm

7aaf853

Addressing @lloydwatkin 's comments on

dd68f8c

#236

Fixing ISO 8601 regex for timestamps with TZ != 0

e1fa775

Removing unnecessary syso

67e95ff

external-domain-checker returning a csv list of local domains

afaf36f

Making it work with Postgres' regex

31ea960

removing unnecessary commented lines

89d4e9c

replacing ~ with regexp_matches in the DatabaseTester level

a7392fd

lloydwatkin reviewed Sep 28, 2014
View reviewed changes

abmargb added 3 commits September 29, 2014 11:53

creates getLocalNodes and getRemoteNodes methods in the node store

78fe33c

Merge remote-tracking branch 'origin/master' into multi-domain-fixes

01ff69b

Conflicts: src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/items/special/FirehoseGet.java

Considering 'local' access model when retrieving /firehose. Various

e7712ad

enhancements as suggested by @lloydwatkin.

lloydwatkin reviewed Oct 3, 2014
View reviewed changes

Moving checks for local domains, jids and nodes to the Configuration

ebdecf9

class.

fixing checkstyle violations

102761c

abmargb added a commit that referenced this pull request Oct 10, 2014

Merge pull request #238 from buddycloud/multi-domain-fixes

60f0952

Fixes related to mutl-domain deployments

abmargb merged commit 60f0952 into master Oct 10, 2014

abmargb deleted the multi-domain-fixes branch October 10, 2014 02:27

This was referenced Oct 13, 2014

server takes forever to connect to the component port #210

Closed

server gets too busy when the crawler kicks in #211

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes related to mutl-domain deployments #238

Fixes related to mutl-domain deployments #238

abmargb commented Sep 27, 2014

lloydwatkin commented Sep 28, 2014

lloydwatkin Sep 28, 2014

abmargb Sep 29, 2014

lloydwatkin Sep 29, 2014

abmargb Sep 29, 2014

lloydwatkin Sep 29, 2014

abmargb Sep 29, 2014

lloydwatkin Oct 3, 2014

abmargb Oct 4, 2014

lloydwatkin commented Oct 3, 2014

abmargb commented Oct 4, 2014

abmargb commented Oct 6, 2014

coveralls commented Oct 9, 2014

coveralls commented Oct 10, 2014

Fixes related to mutl-domain deployments #238

Fixes related to mutl-domain deployments #238

Conversation

abmargb commented Sep 27, 2014

lloydwatkin commented Sep 28, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lloydwatkin commented Oct 3, 2014

abmargb commented Oct 4, 2014

abmargb commented Oct 6, 2014

coveralls commented Oct 9, 2014

coveralls commented Oct 10, 2014