Jordan Birdsell has posted comments on this change.

Change subject: [python] - Update kudu.connect to enable multi-master
......................................................................


Patch Set 5:

(5 comments)

See my comment @L63 in common. This sort of ties into our slack convo 
yesterday. We have to know all of the master addresses before starting any of 
them.

http://gerrit.cloudera.org:8080/#/c/4883/4/python/kudu/__init__.py
File python/kudu/__init__.py:

PS4, Line 89: addresses
> What happens if this is empty?
Good call, it would hang. I updated this but did so in the client constructor 
in case anyone is calling that directly.


http://gerrit.cloudera.org:8080/#/c/4883/4/python/kudu/tests/common.py
File python/kudu/tests/common.py:

PS4, Line 69: 
> This flag makes the server dump info -- including the port it bound to -- t
see above.


PS4, Line 88: 
            : 
> Can you open the server info file for each master and get the port there?
see above


PS4, Line 61: s = socket.socket()
            :             s.bind(('', 0))
            :             master_ports.append(s.getsockname()[1
> I don't think we need to do this. The code now gets the bound port by readi
This doesn't work because you need to know the host/port of every master at the 
time that any one of them is started (-master-addresses). Otherwise, you just 
end up with a bunch of masters that aren't communicating with one another. (RE: 
our slack conversation yesterday). If we could alter that option at runtime it 
would be fine but it seems we cannot.


PS4, Line 64: s.close()
> The socket module docs mention it's best to call socket.shutdown before soc
normally this would be the case, but since we never started listening on this 
socket shutdown() will throw.  Since shutdown just ensures we stop 
communications, i think we're safe not doing it here as it wouldn't be possible 
to have any communication.


-- 
To view, visit http://gerrit.cloudera.org:8080/4883
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2721236d5f92ced2afb4a867511c4144a2ab16a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jordan Birdsell <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Jordan Birdsell <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Will Berkeley <[email protected]>
Gerrit-HasComments: Yes

Reply via email to