giraph-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Maja Kabiljo" <majakabi...@fb.com>
Subject Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Date Mon, 03 Sep 2012 16:36:33 GMT


> On Aug. 30, 2012, 3:34 a.m., Eugene Koontz wrote:
> >

Eugene, this seems like a lot of great work, I'm grateful I was advised against trying to
solve this issue when I first started :-)

When I run "mvn -Phadoop_2.0.1 clean test" I get: 
The requested profile "hadoop_2.0.1" could not be activated because it does not exist
And with other profiles this code doesn't compile.

When merging with GIRAPH-313 you reverted a few things which were changed there. I removed
ServerData from RequestServerHandler and made separate handlers for worker and master with
the idea that they will have different kinds of data and therefore different methods which
can be called on it. Since right now master communication is not used yet you didn't have
any problems with the tests, and it seems to me this addition won't work well with it. I suggest
putting secretManager somewhere else, or we could make ServerData abstract and keep the secretManager
there and then make two implementations of it, one for worker with current stuff. Also you
should return to calling processRequest instead of request.doRequest in RequestServerHandler.
Or if you have some better suggestion on how to have different requests work on different
data.


- Maja


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6609/#review10873
-----------------------------------------------------------


On Aug. 30, 2012, 1:50 a.m., Eugene Koontz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6609/
> -----------------------------------------------------------
> 
> (Updated Aug. 30, 2012, 1:50 a.m.)
> 
> 
> Review request for giraph.
> 
> 
> Description
> -------
> 
> Same patch as: https://issues.apache.org/jira/secure/attachment/12543007/GIRAPH-211.patch
> 
> Current limitations:
> 
> -Not tested on other than hadoop 2.0.1-SNAPSHOT
> -Needs useAuth=true switch to turn on/off (default = off(?))
> 
> How to compile and test:
> 
> mvn -Phadoop_2.0.1 clean test
> 
> Works with the following test:
> 
> $HADOOP_RUNTIME/bin/hadoop jar $GIRAPH_DIR/target/giraph-0.2-SNAPSHOT-for-hadoop-2.0.1-SNAPSHOT-jar-with-dependencies.jar
org.apache.giraph.benchmark.PageRankBenchmark -Dgiraph.useNetty=true -e 2 -s 10 -v -V 2 -w
2
> 
> 
> This addresses bug GIRAPH-211.
>     https://issues.apache.org/jira/browse/GIRAPH-211
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java bc53b83 
>   src/main/java/org/apache/giraph/comm/ServerData.java 68a2044 
>   src/main/java/org/apache/giraph/comm/WorkerClient.java c3ec4fe 
>   src/main/java/org/apache/giraph/comm/netty/NettyClient.java 428b877 
>   src/main/java/org/apache/giraph/comm/netty/NettyServer.java 5904908 
>   src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java 72e79a9 
>   src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientServer.java b06e0f6 
>   src/main/java/org/apache/giraph/comm/netty/SaslNettyClient.java PRE-CREATION 
>   src/main/java/org/apache/giraph/comm/netty/SaslNettyServer.java PRE-CREATION 
>   src/main/java/org/apache/giraph/comm/netty/handler/MasterRequestServerHandler.java
719515d 
>   src/main/java/org/apache/giraph/comm/netty/handler/RequestServerHandler.java 6f0fc20

>   src/main/java/org/apache/giraph/comm/netty/handler/ResponseClientHandler.java c1c2574

>   src/main/java/org/apache/giraph/comm/netty/handler/WorkerRequestServerHandler.java
e8b6c54 
>   src/main/java/org/apache/giraph/comm/requests/NullReply.java PRE-CREATION 
>   src/main/java/org/apache/giraph/comm/requests/RequestType.java 4fbf692 
>   src/main/java/org/apache/giraph/comm/requests/ResponseEncoder.java PRE-CREATION 
>   src/main/java/org/apache/giraph/comm/requests/SaslComplete.java PRE-CREATION 
>   src/main/java/org/apache/giraph/comm/requests/SaslTokenMessage.java PRE-CREATION 
>   src/main/java/org/apache/giraph/comm/requests/SendPartitionCurrentMessagesRequest.java
7cae1e2 
>   src/main/java/org/apache/giraph/comm/requests/SendPartitionMessagesRequest.java 6c7ebb6

>   src/main/java/org/apache/giraph/comm/requests/SendPartitionMutationsRequest.java d3553c1

>   src/main/java/org/apache/giraph/comm/requests/SendVertexRequest.java 2004db4 
>   src/main/java/org/apache/giraph/comm/requests/WritableRequest.java a68b26a 
> 
> Diff: https://reviews.apache.org/r/6609/diff/
> 
> 
> Testing
> -------
> 
> Tested only on Hadoop 2.0.1-SNAPSHOT : needs testing on other Hadoops.
> 
> 
> Thanks,
> 
> Eugene Koontz
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message