search

pforpallav / cluster-server

A cluster server in go-lang
2 stars 0 forks source link

Review for assignment 2. #1

Closed sriram-srinivasan closed 10 years ago

sriram-srinivasan commented 10 years ago

Hi Kumar,

This is a review of your assignement 2.

  1. I have the latest zeromq installed, and alecthomas's zmq library is not up-to-date, so I have not been able to test your submission. Please use the github.com/pebbe/zmq4 library instead.
  2. Creating a new socket and connecting to it (in Sender) for every single message is extremely inefficient. Please refer to my notes on Piazza about assignment 2. Also don't use REQ/REP sockets.
  3. You are doing an all-to-all send / receive test, which is a good thing. But that is precisely what the broadcast Pid in the envelope is for. The client should not have to enumerate each peer in the cluster.
  4. The test strategy may not work if you change the socket type (which you must). You expect the messages to be received in the same order that they were sent in. Shouldn't have that expectation.
  5. Reciever is misspelt. It is Receiver. Here's the rule: I before E except C. Also "AddPear" in README
pforpallav commented 10 years ago

Sir, I have updated the zmq binding to pebbe/zmq4. You should be able to run now. I have not yet dealt with the 2nd point but will do it soon. All other issues mentioned have been addressed.

sriram-srinivasan commented 10 years ago

Wow, some dedication!

Yes, 2 needs to be addressed because my machine complains about "too many open files" (at NewSocket)

pforpallav commented 10 years ago

Tests were PASS-ing on my machine. Yet, I have addressed the 2nd point and avoided re-creating PUSH sockets. It should be fine now.

sriram-srinivasan commented 10 years ago

Good job! Works for me now.