Skip to content

Commit

Permalink
JCBC-123: Throw an exception when vbucket master is -1
Browse files Browse the repository at this point in the history
This changeset makes sure to throw a controlled exception when
vor the given vbucket there is no master server defined to handle
it appropriately. -1 for the master is clearly an invalid state
and could be the case for the following reason:

- No replicas are defined and a node is failed over.
- More nodes have been failed over than there are replicas.

Either way, the client library is unable to remedy the situation
on its on and therefore throws an exception. The application
layer is expected to deal with the situation (either retry until
a rebalance is done) or redirect it to a system which is capable
to do so.

Change-Id: I582939820ec3067ce724c93e410e93f834c340ee
Reviewed-on: http://review.couchbase.org/22352
Reviewed-by: Matt Ingenthron <matt@couchbase.com>
Reviewed-by: Michael Wiederhold <mike@couchbase.com>
Tested-by: Michael Nitschinger <michael.nitschinger@couchbase.com>
  • Loading branch information
daschl authored and Michael Nitschinger committed Nov 9, 2012
1 parent a0d9738 commit 66b8f32
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 1 deletion.
Expand Up @@ -71,6 +71,15 @@ public MemcachedNode getPrimary(String k) {
Map<String, MemcachedNode> nodesMap = totConfig.getNodesMap();
int vbucket = config.getVbucketByKey(k);
int serverNumber = config.getMaster(vbucket);

if(serverNumber == -1) {
throw new RuntimeException("The key "+ k +" pointed to vbucket "+ vbucket
+ ", for which no server is responsible in the cluster map. This "
+ "can be an indication that either no replica is defined for a "
+ "failed server or more nodes have been failed over than replicas "
+ "defined.");
}

String server = config.getServer(serverNumber);
// choose appropriate MemcachedNode according to config data
MemcachedNode pNode = nodesMap.get(server);
Expand Down
Expand Up @@ -30,7 +30,7 @@
import java.util.Arrays;

import junit.framework.TestCase;

import org.junit.Test;
import net.spy.memcached.MemcachedNode;

import static org.easymock.EasyMock.createMock;
Expand Down Expand Up @@ -62,6 +62,24 @@ public class VBucketNodeLocatorTest extends TestCase {
+ " [1, 2, 0],\n" + " [2, 1, -1],\n" + " [1, 2, 0]\n"
+ " ]\n" + "}" + "}";

private static final String NO_REPLICA_CONFIG_IN_ENVELOPE =
"{ \"otherKeyThatIsIgnored\": 12345,\n"
+ "\"nodes\": [\n"
+ "{\n"
+ "\"clusterCompatibility\": 1,\n"
+ "\"clusterMembership\": \"active\"\n,"
+ "\"couchApiBase\": \"http://10.2.1.67:5984/\"\n"
+ "}\n"
+ "],\n"
+ "\"vBucketServerMap\": \n"
+ "{\n"
+ " \"hashAlgorithm\": \"CRC\",\n"
+ " \"numReplicas\": 0,\n"
+ " \"serverList\": [\"127.0.0.1:11211\", \"127.0.0.1:11210\"],\n"
+ " \"vBucketMap\":\n" + " [\n" + " [-1],\n"
+ " [-1],\n" + " [0],\n" + " [0]\n"
+ " ]\n" + "}" + "}";

public void testGetPrimary() {
MemcachedNode node1 = createMock(MemcachedNode.class);
MemcachedNode node2 = createMock(MemcachedNode.class);
Expand Down Expand Up @@ -104,4 +122,27 @@ public void testGetAlternative() {
locator.getAlternative("k1", Arrays.asList(primary));
alternative.getSocketAddress();
}

@Test
public void testNoMasterServerForVbucket() {
MemcachedNodeMockImpl node1 = new MemcachedNodeMockImpl();
MemcachedNodeMockImpl node2 = new MemcachedNodeMockImpl();
node1.setSocketAddress(new InetSocketAddress("127.0.0.1", 11211));
node2.setSocketAddress(new InetSocketAddress("127.0.0.1", 11210));

ConfigFactory configFactory = new DefaultConfigFactory();
Config config = configFactory.create(NO_REPLICA_CONFIG_IN_ENVELOPE);

VBucketNodeLocator locator = new VBucketNodeLocator(
Arrays.asList((MemcachedNode) node1, node2),
config);

boolean success = false;
try {
MemcachedNode primary = locator.getPrimary("key1");
} catch(RuntimeException e) {
success = true;
}
assertTrue("No RuntimeException thrown when vbucket master is -1", success);
}
}

0 comments on commit 66b8f32

Please sign in to comment.