-
Notifications
You must be signed in to change notification settings - Fork 8
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
Parse GRPC data in ZooKeeper. #14
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import ramcloud | ||
import os | ||
import ramcloud | ||
import Table_pb2 | ||
import unittest | ||
from pyexpect import expect | ||
import cluster_test_utils as ctu | ||
|
@@ -24,19 +25,19 @@ def createTestValue(self): | |
def make_cluster(self, num_nodes): | ||
self.assertGreaterEqual(num_nodes, 3) | ||
|
||
ensemble = {i: '10.0.1.{}'.format(i) for i in xrange(1, num_nodes + 1)} | ||
zk_servers = ctu.ensemble_servers_string(ensemble) | ||
external_storage = ctu.external_storage_string(ensemble) | ||
self.ensemble = {i: '10.0.1.{}'.format(i) for i in xrange(1, num_nodes + 1)} | ||
zk_servers = ctu.ensemble_servers_string(self.ensemble) | ||
external_storage = 'zk:' + ctu.external_storage_string(self.ensemble) | ||
for i in xrange(1, num_nodes + 1): | ||
hostname = 'ramcloud-node-{}'.format(i) | ||
self.node_containers[ensemble[i]] = ctu.launch_node('main', | ||
hostname, | ||
zk_servers, | ||
external_storage, | ||
i, | ||
ensemble[i], | ||
self.node_image, | ||
self.ramcloud_network) | ||
self.node_containers[self.ensemble[i]] = ctu.launch_node('main', | ||
hostname, | ||
zk_servers, | ||
external_storage, | ||
i, | ||
self.ensemble[i], | ||
self.node_image, | ||
self.ramcloud_network) | ||
self.rc_client.connect(external_storage, 'main') | ||
|
||
def simple_recovery(self, kill_command): | ||
|
@@ -56,6 +57,20 @@ def simple_recovery(self, kill_command): | |
value = self.rc_client.read(self.table, 'testKey') | ||
expect(value).equals(('testValue', 1)) | ||
|
||
def test_zookeeper_read(self): | ||
self.make_cluster(num_nodes=4) | ||
self.createTestValue() | ||
zk_client = ctu.get_zookeeper_client(self.ensemble) | ||
|
||
# Read the ZooKeeper entry for the table and make sure it looks sane. | ||
# This mostly tests our ability to read from ZooKeeper and parse the | ||
# GRPC contents correctly. | ||
table_data = zk_client.get('/ramcloud/main/tables/test')[0] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not entirely sure why you're using '/ramcloud/main/tables/test' for table_data. I'm sure there's a good reason. Can I ask you to clarify this in the comment? |
||
table_parsed = Table_pb2.Table() | ||
table_parsed.ParseFromString(table_data) | ||
expect(table_parsed.id).equals(1L) | ||
expect(table_parsed.name).equals("test") | ||
|
||
def test_read_write(self): | ||
self.make_cluster(num_nodes=3) | ||
self.rc_client.create_table('test_table') | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that, with your change, some calls need the 'zk:' prefix, and other calls do not. but moving the 'zk:' string over to this file for concatenation feels a bit tacky. Wouldn't it be cleaner to simply add a boolean parameter use_zk_prefix for the ctu.external_storage_string() method?