You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@giraph.apache.org by Eugene Koontz <ek...@hiro-tan.org> on 2012/08/14 19:10:28 UTC
Review Request: GIRAPH-211: Add secure authentication to Netty IPC
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6609/
-----------------------------------------------------------
Review request for giraph.
Description
-------
Same patch as https://issues.apache.org/jira/secure/attachment/12539928/GIRAPH-211.patch
Current limitations:
-Authorization is not done: that is, clients are authenticated but there are no restrictions on their ability to do RPC on the servers.
-Clients should wait for authentication before trying to do RPC - once authorization (see above) is done, they might encounter a race where they try to do RPCs without yet being authenticated.
-Not tested on other than hadoop 2.0.1-SNAPSHOT
-Only works if we disable client-side channel-pooling (GIRAPH-289) and local short-circuiting of RPCs (GIRAPH-262) - these should be configurable but currently, I hard-wired both to be disabled.
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
-----
pom.xml c4ea04f
src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java f103c7e
src/main/java/org/apache/giraph/comm/ClientData.java PRE-CREATION
src/main/java/org/apache/giraph/comm/CommunicationsInterface.java d60f512
src/main/java/org/apache/giraph/comm/NettyClient.java fe196df
src/main/java/org/apache/giraph/comm/NettyServer.java 7a658fa
src/main/java/org/apache/giraph/comm/NettyWorkerClient.java 40e5665
src/main/java/org/apache/giraph/comm/NettyWorkerServer.java 8c44f11
src/main/java/org/apache/giraph/comm/NullReply.java PRE-CREATION
src/main/java/org/apache/giraph/comm/PeerMessage.java PRE-CREATION
src/main/java/org/apache/giraph/comm/RPCCommunications.java 945a92e
src/main/java/org/apache/giraph/comm/RequestDecoder.java cd930b2
src/main/java/org/apache/giraph/comm/RequestEncoder.java bdbd051
src/main/java/org/apache/giraph/comm/RequestRegistry.java 24307b3
src/main/java/org/apache/giraph/comm/RequestServerHandler.java eb851c3
src/main/java/org/apache/giraph/comm/ResponseClientHandler.java c64bf24
src/main/java/org/apache/giraph/comm/ResponseDecoder.java PRE-CREATION
src/main/java/org/apache/giraph/comm/ResponseEncoder.java PRE-CREATION
src/main/java/org/apache/giraph/comm/SaslComplete.java PRE-CREATION
src/main/java/org/apache/giraph/comm/SaslNettyClient.java PRE-CREATION
src/main/java/org/apache/giraph/comm/SaslNettyServer.java PRE-CREATION
src/main/java/org/apache/giraph/comm/SaslTokenMessage.java PRE-CREATION
src/main/java/org/apache/giraph/comm/SendMutationsCache.java b7c3022
src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java c6220fe
src/main/java/org/apache/giraph/comm/SendPartitionMutationsRequest.java 1d31497
src/main/java/org/apache/giraph/comm/SendVertexRequest.java 657a90a
src/main/java/org/apache/giraph/comm/ServerData.java c1eb737
src/main/java/org/apache/giraph/comm/WritableRequest.java 3c87d22
src/main/java/org/apache/giraph/comm/messages/SendPartitionCurrentMessagesRequest.java aaf73ee
src/test/java/org/apache/giraph/comm/ConnectionTest.java b03f244
src/test/java/org/apache/giraph/comm/RequestTest.java 3b0635b
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
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Maja Kabiljo <ma...@fb.com>.
> 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
>
>
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eli Reisman <in...@gmail.com>.
I love the idea of the pluggable ChannelHandler, Netty is really perfectly
set up for this, nice call!
On Wed, Sep 5, 2012 at 2:31 AM, Maja Kabiljo <ma...@fb.com> wrote:
> Eugene, maybe there is something wrong with the way I set up the project
> then. Here are the errors I'm getting:
> - With hadoop_2.0.0 no method TokenIdentifier.decodeIdentifier
> - With hadoop_0.23 can't find org.apache.commons.net.util.Base64, no
> method JobTokenSecretManager.checkAvailableForRead and the one from above
> What am I doing wrong?
>
> Looking forward to see the updated patch!
>
>
> Eli, I see that the confusion arises because I left
> MasterRequest.doRequest without parameters, which I did just because it's
> not used yet. But it's going to have to accept some master data once we
> have implementations of MasterRequest. The only way I see to keep only one
> doRequest signature in the end is to have some ServerData which has
> methods for accessing both master and worker stuff, and say throws an
> exception in unsupported method on each. But that seems ugly, and also
> makes us keep generics on places where we don't need them. I just have a
> feeling there should be a nice way to solve this which I'm missing because
> I'm too focused on current solution ;-)
>
> Maja
>
> On 9/5/12 2:56 AM, "Avery Ching" <ac...@apache.org> wrote:
>
> >I really like this idea, so we don't need to have a separate
> >SaslNettyClient and SaslNettyServer. It makes a lot of sense to have a
> >ChannelHandler that can be inserted if security is desired. This should
> >be a lot cleaner.
> >
> >Avery
> >
> >On 9/4/12 11:31 AM, Eugene Koontz wrote:
> >> Thanks Maja and Eli for your comments.
> >>
> >> I'm glad you are looking at it and discussing it! It's been a real
> >> learning experience for me too.
> >>
> >> I am working today on a new iteration of the patch which introduces a
> >> dedicated ChannelHandler dedicated to SASL- that is, a component in the
> >> pipelines of clients and servers that would be added only if SASL
> >> authentication is enabled by configuration, and is removed after
> >> authentication is completed.
> >>
> >> Adding and removing this handler can be done using
> >> ChannelPipeline.addAfter() and .remove(), respectively:
> >>
> >>
> >>
> http://docs.jboss.org/netty/3.2/api/org/jboss/netty/channel/ChannelPipeli
> >>ne.html#addFirst%28%29
> >>
> >> I think this would better localize SASL-specific communication,
> >> touching fewer existing files, and slim down the patch. Hopefully this
> >> would also eliminate the need to pass around the ServerData object, and
> >> avoid signature changes to doRequest().
> >>
> >> With regard to -Phadoop_2.0.1, sorry about that, Maja - I will open a
> >> new JIRA to add this profile to pom.xml.
> >>
> >> The patch should work, however, with other hadoop profiles, as long as
> >> they don't have HADOOP_NON_SECURE in their munge flags.
> >>
> >> -Eugene
> >
>
>
Re: Review Request: GIRAPH-211: Add secure authentication to Netty
IPC
Posted by Maja Kabiljo <ma...@fb.com>.
Eugene, maybe there is something wrong with the way I set up the project
then. Here are the errors I'm getting:
- With hadoop_2.0.0 no method TokenIdentifier.decodeIdentifier
- With hadoop_0.23 can't find org.apache.commons.net.util.Base64, no
method JobTokenSecretManager.checkAvailableForRead and the one from above
What am I doing wrong?
Looking forward to see the updated patch!
Eli, I see that the confusion arises because I left
MasterRequest.doRequest without parameters, which I did just because it's
not used yet. But it's going to have to accept some master data once we
have implementations of MasterRequest. The only way I see to keep only one
doRequest signature in the end is to have some ServerData which has
methods for accessing both master and worker stuff, and say throws an
exception in unsupported method on each. But that seems ugly, and also
makes us keep generics on places where we don't need them. I just have a
feeling there should be a nice way to solve this which I'm missing because
I'm too focused on current solution ;-)
Maja
On 9/5/12 2:56 AM, "Avery Ching" <ac...@apache.org> wrote:
>I really like this idea, so we don't need to have a separate
>SaslNettyClient and SaslNettyServer. It makes a lot of sense to have a
>ChannelHandler that can be inserted if security is desired. This should
>be a lot cleaner.
>
>Avery
>
>On 9/4/12 11:31 AM, Eugene Koontz wrote:
>> Thanks Maja and Eli for your comments.
>>
>> I'm glad you are looking at it and discussing it! It's been a real
>> learning experience for me too.
>>
>> I am working today on a new iteration of the patch which introduces a
>> dedicated ChannelHandler dedicated to SASL- that is, a component in the
>> pipelines of clients and servers that would be added only if SASL
>> authentication is enabled by configuration, and is removed after
>> authentication is completed.
>>
>> Adding and removing this handler can be done using
>> ChannelPipeline.addAfter() and .remove(), respectively:
>>
>>
>>http://docs.jboss.org/netty/3.2/api/org/jboss/netty/channel/ChannelPipeli
>>ne.html#addFirst%28%29
>>
>> I think this would better localize SASL-specific communication,
>> touching fewer existing files, and slim down the patch. Hopefully this
>> would also eliminate the need to pass around the ServerData object, and
>> avoid signature changes to doRequest().
>>
>> With regard to -Phadoop_2.0.1, sorry about that, Maja - I will open a
>> new JIRA to add this profile to pom.xml.
>>
>> The patch should work, however, with other hadoop profiles, as long as
>> they don't have HADOOP_NON_SECURE in their munge flags.
>>
>> -Eugene
>
Re: Review Request: GIRAPH-211: Add secure authentication to Netty
IPC
Posted by Avery Ching <ac...@apache.org>.
I really like this idea, so we don't need to have a separate
SaslNettyClient and SaslNettyServer. It makes a lot of sense to have a
ChannelHandler that can be inserted if security is desired. This should
be a lot cleaner.
Avery
On 9/4/12 11:31 AM, Eugene Koontz wrote:
> Thanks Maja and Eli for your comments.
>
> I'm glad you are looking at it and discussing it! It's been a real
> learning experience for me too.
>
> I am working today on a new iteration of the patch which introduces a
> dedicated ChannelHandler dedicated to SASL- that is, a component in the
> pipelines of clients and servers that would be added only if SASL
> authentication is enabled by configuration, and is removed after
> authentication is completed.
>
> Adding and removing this handler can be done using
> ChannelPipeline.addAfter() and .remove(), respectively:
>
> http://docs.jboss.org/netty/3.2/api/org/jboss/netty/channel/ChannelPipeline.html#addFirst%28%29
>
> I think this would better localize SASL-specific communication,
> touching fewer existing files, and slim down the patch. Hopefully this
> would also eliminate the need to pass around the ServerData object, and
> avoid signature changes to doRequest().
>
> With regard to -Phadoop_2.0.1, sorry about that, Maja - I will open a
> new JIRA to add this profile to pom.xml.
>
> The patch should work, however, with other hadoop profiles, as long as
> they don't have HADOOP_NON_SECURE in their munge flags.
>
> -Eugene
Re: Review Request: GIRAPH-211: Add secure authentication to Netty
IPC
Posted by Eugene Koontz <ek...@hiro-tan.org>.
Thanks Maja and Eli for your comments.
I'm glad you are looking at it and discussing it! It's been a real
learning experience for me too.
I am working today on a new iteration of the patch which introduces a
dedicated ChannelHandler dedicated to SASL- that is, a component in the
pipelines of clients and servers that would be added only if SASL
authentication is enabled by configuration, and is removed after
authentication is completed.
Adding and removing this handler can be done using
ChannelPipeline.addAfter() and .remove(), respectively:
http://docs.jboss.org/netty/3.2/api/org/jboss/netty/channel/ChannelPipeline.html#addFirst%28%29
I think this would better localize SASL-specific communication,
touching fewer existing files, and slim down the patch. Hopefully this
would also eliminate the need to pass around the ServerData object, and
avoid signature changes to doRequest().
With regard to -Phadoop_2.0.1, sorry about that, Maja - I will open a
new JIRA to add this profile to pom.xml.
The patch should work, however, with other hadoop profiles, as long as
they don't have HADOOP_NON_SECURE in their munge flags.
-Eugene
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eli Reisman <in...@gmail.com>.
> On Aug. 30, 2012, 3:34 a.m., Eugene Koontz wrote:
> >
>
> Maja Kabiljo 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.
>
> Eli Reisman wrote:
> Yeah it looks like the patch might have gone a bit stale. This is great work though, I am learning a lot about SASL and our security model just reading it! I'll be happy to come back and try to review this properly when the patch is rebased.
>
> As for the doRequest/process request thing, I meant to ask about that change before. Seems like its a good pattern for WritableRequests to meet a common doRequest-style interface and let whatever any subclass of WritableRequest needs to do happen in there (like the Command pattern.) It does avoid the unneeded generics and avoids putting the responsibility on the Handler's processRequest() to figure out what each request needs to do, what type it is, and how to do it. Letting the Handler just call a known API like doRequest (or call it processRequest) implemented at the Request side and supplied with the objects it needs to have side effects on keeps each code path separate in its own WritableRequest object. As long as there's a big rebase to do here anyway, maybe it wouldn't be bad if couple things like that could get reinstated in some sensible way?
>
> Either way, great work. This is crucial for getting rid of the old RPC and I'll be excited to see this in the codebase!
>
>
> Eli Reisman wrote:
> Never mind, I see doRequest is still in there in the subclasses of the Handlers. So processRequest is like this because of the new master communication channels, and the master one doesn't get a ServerData as input? Fair enough. So Maja: you're saying the trouble for this patch is that the master requests don't' get access to the SeverData but the current version of the patch expects one to be there in a request?
>
>
>
> Maja Kabiljo wrote:
> Eli, I just mentioned this because Eugene was saying he merged with 313.
>
> If I understand this patch correctly, it's expected that we have ServerData object in order to be able to do secure communication, and on master side it's set to null (and we also shouldn't have ServerData object in its current state on master because it has stuff master shouldn't have).
>
> As for processRequest/doRequest, honestly, I'm not very happy with the way I solved it. Reasons being that even though we separated the data we have on worker and on master, for worker requests which don't deal with graph data we'll still have to have generics so we would be able to have doRequest(ServerData<I,V,E,M>). I couldn't figure out how to solve that in a nice way, and still have RequestHandler treating all requests in the same way. It's like we want to have several different doRequest signatures. I'm really interested in your thoughts about this.
> (Sorry for spamming this patch, probably we should move this discussion elsewhere)
I see. Yeah there's no happy solution there if we need to change the doRequest() signature. You did a great job on the message storage, I have been trying them out lately. If it keeps all the WritableRequest signatures the same, maybe it would be OK to pass "null" to doRequest for a MasterRequest, as ugly as that is, and just document it?
In regards to getting this patch to work, is there anything we can do to help Eugene get the data he needs integrated into the messaging as we have it now, without ServerData being uniformly available? I feel like you're (Maja) probably in the best position to know what the tradeoffs are in that regard.
- Eli
-----------------------------------------------------------
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
>
>
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Maja Kabiljo <ma...@fb.com>.
> On Aug. 30, 2012, 3:34 a.m., Eugene Koontz wrote:
> >
>
> Maja Kabiljo 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.
>
> Eli Reisman wrote:
> Yeah it looks like the patch might have gone a bit stale. This is great work though, I am learning a lot about SASL and our security model just reading it! I'll be happy to come back and try to review this properly when the patch is rebased.
>
> As for the doRequest/process request thing, I meant to ask about that change before. Seems like its a good pattern for WritableRequests to meet a common doRequest-style interface and let whatever any subclass of WritableRequest needs to do happen in there (like the Command pattern.) It does avoid the unneeded generics and avoids putting the responsibility on the Handler's processRequest() to figure out what each request needs to do, what type it is, and how to do it. Letting the Handler just call a known API like doRequest (or call it processRequest) implemented at the Request side and supplied with the objects it needs to have side effects on keeps each code path separate in its own WritableRequest object. As long as there's a big rebase to do here anyway, maybe it wouldn't be bad if couple things like that could get reinstated in some sensible way?
>
> Either way, great work. This is crucial for getting rid of the old RPC and I'll be excited to see this in the codebase!
>
>
> Eli Reisman wrote:
> Never mind, I see doRequest is still in there in the subclasses of the Handlers. So processRequest is like this because of the new master communication channels, and the master one doesn't get a ServerData as input? Fair enough. So Maja: you're saying the trouble for this patch is that the master requests don't' get access to the SeverData but the current version of the patch expects one to be there in a request?
>
>
Eli, I just mentioned this because Eugene was saying he merged with 313.
If I understand this patch correctly, it's expected that we have ServerData object in order to be able to do secure communication, and on master side it's set to null (and we also shouldn't have ServerData object in its current state on master because it has stuff master shouldn't have).
As for processRequest/doRequest, honestly, I'm not very happy with the way I solved it. Reasons being that even though we separated the data we have on worker and on master, for worker requests which don't deal with graph data we'll still have to have generics so we would be able to have doRequest(ServerData<I,V,E,M>). I couldn't figure out how to solve that in a nice way, and still have RequestHandler treating all requests in the same way. It's like we want to have several different doRequest signatures. I'm really interested in your thoughts about this.
(Sorry for spamming this patch, probably we should move this discussion elsewhere)
- 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
>
>
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eli Reisman <in...@gmail.com>.
> On Aug. 30, 2012, 3:34 a.m., Eugene Koontz wrote:
> >
>
> Maja Kabiljo 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.
Yeah it looks like the patch might have gone a bit stale. This is great work though, I am learning a lot about SASL and our security model just reading it! I'll be happy to come back and try to review this properly when the patch is rebased.
As for the doRequest/process request thing, I meant to ask about that change before. Seems like its a good pattern for WritableRequests to meet a common doRequest-style interface and let whatever any subclass of WritableRequest needs to do happen in there (like the Command pattern.) It does avoid the unneeded generics and avoids putting the responsibility on the Handler's processRequest() to figure out what each request needs to do, what type it is, and how to do it. Letting the Handler just call a known API like doRequest (or call it processRequest) implemented at the Request side and supplied with the objects it needs to have side effects on keeps each code path separate in its own WritableRequest object. As long as there's a big rebase to do here anyway, maybe it wouldn't be bad if couple things like that could get reinstated in some sensible way?
Either way, great work. This is crucial for getting rid of the old RPC and I'll be excited to see this in the codebase!
- Eli
-----------------------------------------------------------
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
>
>
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eli Reisman <in...@gmail.com>.
> On Aug. 30, 2012, 3:34 a.m., Eugene Koontz wrote:
> >
>
> Maja Kabiljo 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.
>
> Eli Reisman wrote:
> Yeah it looks like the patch might have gone a bit stale. This is great work though, I am learning a lot about SASL and our security model just reading it! I'll be happy to come back and try to review this properly when the patch is rebased.
>
> As for the doRequest/process request thing, I meant to ask about that change before. Seems like its a good pattern for WritableRequests to meet a common doRequest-style interface and let whatever any subclass of WritableRequest needs to do happen in there (like the Command pattern.) It does avoid the unneeded generics and avoids putting the responsibility on the Handler's processRequest() to figure out what each request needs to do, what type it is, and how to do it. Letting the Handler just call a known API like doRequest (or call it processRequest) implemented at the Request side and supplied with the objects it needs to have side effects on keeps each code path separate in its own WritableRequest object. As long as there's a big rebase to do here anyway, maybe it wouldn't be bad if couple things like that could get reinstated in some sensible way?
>
> Either way, great work. This is crucial for getting rid of the old RPC and I'll be excited to see this in the codebase!
>
Never mind, I see doRequest is still in there in the subclasses of the Handlers. So processRequest is like this because of the new master communication channels, and the master one doesn't get a ServerData as input? Fair enough. So Maja: you're saying the trouble for this patch is that the master requests don't' get access to the SeverData but the current version of the patch expects one to be there in a request?
- Eli
-----------------------------------------------------------
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
>
>
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eugene Koontz <ek...@hiro-tan.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6609/#review10873
-----------------------------------------------------------
src/main/java/org/apache/giraph/comm/netty/SaslNettyServer.java
<https://reviews.apache.org/r/6609/#comment23433>
Remove this line
- Eugene Koontz
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
>
>
Re: Review Request: GIRAPH-211: Add secure authentication to Netty
IPC
Posted by Eugene Koontz <ek...@hiro-tan.org>.
On 9/27/12 4:51 PM, Avery Ching wrote:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6609/#review12009
> -----------------------------------------------------------
>
>
> Thanks Eugene for continuing this. I think you may have missed my comments from the JIRA as many of them are not addressed here. Also this appears to not be rebased on trunk.
>
>
Hi Avery,
I did see your comments, and appreciate them and will address them.
Today I was also adding some additional notes to myself.
-Eugene
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Avery Ching <av...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6609/#review12009
-----------------------------------------------------------
Thanks Eugene for continuing this. I think you may have missed my comments from the JIRA as many of them are not addressed here. Also this appears to not be rebased on trunk.
Here is what I wrote earlier (some maybe not apply anymore):
https://issues.apache.org/jira/browse/GIRAPH-211?focusedCommentId=13459279&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13459279
--------------------------------------
Eugene, this looks nice! I still see some checkstyle stuff but you're working through it I guess. Overall, the design is good. If we think about improvements we can make them later on.
Questions/Comments:
For consistency, can you please convert multi-line comments like
/** Whether to use SASL with DIGEST and Hadoop Job Tokens to authenticate
* and authorize Netty BSP Clients to Servers. */
to
/**
* Whether to use SASL with DIGEST and Hadoop Job Tokens to authenticate
* and authorize Netty BSP Clients to Servers.
*/
Can we get rename Authorize to AuthorizeServerHandler or something else more descriptive?
NettyClient.java
372: Please wrap the LOG.info() with if (LOG.isInfoEnabled()).
545-557: Can't we just go through the regular netty request part of the code? We don't need to have -2 here and can just submit the destWorkerId?
SASL_COMPLETE -> SASL_COMPLETE_REQUEST?
SaslTokenMessage.java can we call is SaslTokenMessageRequest?
SaslComplete.java can we call it SaslCompleteRequest to match the other names?
SaslComplete.java
29-34: Why not get rid of these?
SaslTokenMessage.java:
86: Extra line
Thanks again, this was a lot of work!
src/main/java/org/apache/giraph/comm/ServerData.java
<https://reviews.apache.org/r/6609/#comment25597>
Please wrap all debug/info level logging i.e. "if (LOG.isDebug...".
src/main/java/org/apache/giraph/comm/ServerData.java
<https://reviews.apache.org/r/6609/#comment25598>
if (LOG.isDebug...,
src/main/java/org/apache/giraph/comm/ServerData.java
<https://reviews.apache.org/r/6609/#comment25599>
if (LOG.isDebug...,
src/main/java/org/apache/giraph/comm/ServerData.java
<https://reviews.apache.org/r/6609/#comment25600>
if (LOG.isDebug...,
src/main/java/org/apache/giraph/comm/ServerData.java
<https://reviews.apache.org/r/6609/#comment25601>
if (LOG.isDebug...,
src/main/java/org/apache/giraph/comm/ServerData.java
<https://reviews.apache.org/r/6609/#comment25602>
if (LOG.isDebug...,
src/main/java/org/apache/giraph/comm/ServerData.java
<https://reviews.apache.org/r/6609/#comment25603>
Is this okay? Or should we exit everything?
src/main/java/org/apache/giraph/comm/ServerData.java
<https://reviews.apache.org/r/6609/#comment25604>
if (LOG.isDebug...
src/main/java/org/apache/giraph/comm/netty/NettyClient.java
<https://reviews.apache.org/r/6609/#comment25605>
Is this TODO something that need to be addressed now?
src/main/java/org/apache/giraph/comm/netty/NettyClient.java
<https://reviews.apache.org/r/6609/#comment25612>
Can't we just go through the regular netty request part of the code? We don't need to have -2 here and can just submit the destWorkerId?
src/main/java/org/apache/giraph/comm/netty/handler/ResponseClientHandler.java
<https://reviews.apache.org/r/6609/#comment25607>
Please wrap and for consistency with rest of code, can you use "decode:" here?
src/main/java/org/apache/giraph/comm/netty/handler/ResponseClientHandler.java
<https://reviews.apache.org/r/6609/#comment25606>
For consistency with rest of code, can you use "decode:" here?
src/main/java/org/apache/giraph/comm/netty/handler/ResponseClientHandler.java
<https://reviews.apache.org/r/6609/#comment25608>
Can you wrap here and for consistency with rest of code, can you use "decode:" here?
src/main/java/org/apache/giraph/comm/netty/handler/ResponseClientHandler.java
<https://reviews.apache.org/r/6609/#comment25609>
Please wrap and for consistency with rest of code, can you use "decode:" here?
src/main/java/org/apache/giraph/comm/requests/RequestType.java
<https://reviews.apache.org/r/6609/#comment25610>
For consistency, can we name this SASL_COMPLETE_REQUEST?
src/main/java/org/apache/giraph/graph/BspServiceWorker.java
<https://reviews.apache.org/r/6609/#comment25611>
This needs to be rebased, we don't use GiraphJob for configuration parameters anymore, see GiraphConfiguration.
- Avery Ching
On Sept. 27, 2012, 6:54 p.m., Eugene Koontz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6609/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2012, 6:54 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> Current limitations:
>
> -Not tested on other than hadoop trunk
>
> How to use:
>
> use
>
> -Dgiraph.useNetty=true -Dgiraph.authenticate=true
>
> on your job's command line to use this feature.
>
> How to compile and test:
>
> mvn -Phadoop_trunk clean test
>
> Works with the following test (after applying patch to trunk):
>
> $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 -Dgiraph.authenticate=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/WorkerClient.java c3ec4fe
> src/main/java/org/apache/giraph/comm/netty/NettyClient.java e6a8508
> 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/Authorize.java PRE-CREATION
> src/main/java/org/apache/giraph/comm/netty/handler/ResponseEncoder.java PRE-CREATION
> src/main/java/org/apache/giraph/comm/netty/handler/SaslClientHandler.java PRE-CREATION
> src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java PRE-CREATION
> src/main/java/org/apache/giraph/comm/requests/RequestType.java 4fbf692
> 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/graph/BspServiceWorker.java d926d1c
> src/main/java/org/apache/giraph/graph/GiraphJob.java 342be46
>
> Diff: https://reviews.apache.org/r/6609/diff/
>
>
> Testing
> -------
>
> Tested only on Hadoop Trunk : needs testing on other Hadoops.
>
>
> Thanks,
>
> Eugene Koontz
>
>
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eugene Koontz <ek...@hiro-tan.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6609/
-----------------------------------------------------------
(Updated Oct. 10, 2012, 5:49 p.m.)
Review request for giraph.
Changes
-------
updated description
Description (updated)
-------
Passes
mvn -Phadoop_non_secure clean verify && mvn -Phadoop_0.20.203 clean verify && mvn -Phadoop_1.0 clean verify && mvn -Phadoop0.23 clean verify && mvn -Phadoop_2.0.0 clean verify && mvn -Phadoop_2.0.1 clean verify && mvn -Phadoop_2.0.2 clean verify
How to use:
use
-Dgiraph.useNetty=true -Dgiraph.authenticate=true
on your job's command line to use this feature.
How to compile and test (using Hadoop 2.0.0 for example):
mvn -Phadoop_2.0.0 clean test
$HADOOP_RUNTIME/bin/hadoop jar $GIRAPH_DIR/target/giraph-0.2-SNAPSHOT-for-hadoop-2.0.0-SNAPSHOT-jar-with-dependencies.jar org.apache.giraph.benchmark.PageRankBenchmark -Dgiraph.useNetty=true -Dgiraph.authenticate=true -e 100 -s 100 -v -V 100 -w 6
Look for something similar to the following in the workers' syslogs:
2012-10-10 09:24:49,924 DEBUG [New I/O client worker #1-2] org.apache.giraph.comm.netty.handler.SaslClientHandler: decode: Got a response of type SASL_TOKEN_MESSAGE_REQUEST from server:Eugenes-MacBook-Pro.local/172.16.175.1:30010
2012-10-10 09:24:49,924 DEBUG [New I/O client worker #1-2] org.apache.giraph.comm.netty.handler.SaslClientHandler: handleUpstream: Responding to server's token of length: 40
2012-10-10 09:24:49,924 DEBUG [New I/O client worker #1-2] org.apache.giraph.comm.netty.handler.SaslClientHandler: handleUpstream: Response to server is null: authentication should now be complete.
2012-10-10 09:24:49,925 DEBUG [New I/O client worker #1-2] org.apache.giraph.comm.netty.ByteCounter: handleUpstream: 0 buffer size = 17, total bytes = 207
2012-10-10 09:24:49,925 DEBUG [New I/O client worker #1-2] org.apache.giraph.comm.netty.handler.SaslClientHandler: decode: Got a response of type SASL_COMPLETE_REQUEST from server:Eugenes-MacBook-Pro.local/172.16.175.1:30010
2012-10-10 09:24:49,925 DEBUG [New I/O client worker #1-2] org.apache.giraph.comm.netty.handler.SaslClientHandler: handleUpstream: Server has sent us the SaslComplete message. Allowing normal work to proceed.
2012-10-10 09:24:49,926 DEBUG [main] org.apache.giraph.comm.netty.NettyClient: authenticateOnChannel: Authentication on channel: [id: 0x32486cdd, /172.16.175.1:54768 => Eugenes-MacBook-Pro.local/172.16.175.1:30010] has completed successfully.
..
..
2012-10-10 09:24:56,844 DEBUG [New I/O server worker #1-6] org.apache.giraph.comm.netty.handler.AuthorizeServerHandler: messageReceived: Got class org.apache.giraph.comm.requests.SendWorkerMessagesRequest
2012-10-10 09:24:56,844 DEBUG [New I/O server worker #1-6] org.apache.giraph.comm.netty.handler.AuthorizeServerHandler: messageReceived: authenticated client: FmpvYl8xMzQ5NzE1MTIxNDc1XzAyMDU= is authorized to do request on server.
2012-10-10 09:24:56,844 INFO [main] org.apache.giraph.comm.netty.NettyClient: waitAllRequests: Finished all requests. MBytes/sec sent = 0.3824, MBytes/sec received = 0.0124, MBytesSent = 0.0019, MBytesReceived = 0.0001, ave sent req MBytes = 0.0004, ave received req MBytes = 0, secs waited = 0.0040
2012-10-10 09:24:56,844 DEBUG [New I/O server worker #1-6] org.apache.giraph.comm.netty.handler.RequestServerHandler: messageReceived: Processing client 3, requestId 101, SEND_WORKER_MESSAGES_REQUEST took 30000 ns
This addresses bug GIRAPH-211.
https://issues.apache.org/jira/browse/GIRAPH-211
Diffs
-----
pom.xml 51944fa
src/main/java/org/apache/giraph/GiraphConfiguration.java 797ba6e
src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java 1849504
src/main/java/org/apache/giraph/comm/CommunicationsInterface.java d60f512
src/main/java/org/apache/giraph/comm/SecureRPCCommunications.java af9720e
src/main/java/org/apache/giraph/comm/WorkerClient.java c3ec4fe
src/main/java/org/apache/giraph/comm/netty/NettyClient.java 93f5671
src/main/java/org/apache/giraph/comm/netty/NettyServer.java 886d68b
src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java 5b71401
src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientServer.java 4710f3a
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/RequestEncoder.java 3174447
src/main/java/org/apache/giraph/comm/netty/handler/SaslClientHandler.java PRE-CREATION
src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java PRE-CREATION
src/main/java/org/apache/giraph/comm/requests/RequestType.java b412aa2
src/main/java/org/apache/giraph/comm/requests/SaslCompleteRequest.java PRE-CREATION
src/main/java/org/apache/giraph/comm/requests/SaslTokenMessageRequest.java PRE-CREATION
src/main/java/org/apache/giraph/graph/BspServiceWorker.java bb498ce
src/test/java/org/apache/giraph/comm/SaslConnectionTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/6609/diff/
Testing (updated)
-------
Tested as above.
Thanks,
Eugene Koontz
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eugene Koontz <ek...@hiro-tan.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6609/
-----------------------------------------------------------
(Updated Oct. 10, 2012, 5:44 p.m.)
Review request for giraph.
Changes
-------
Same patch as described in https://issues.apache.org/jira/browse/GIRAPH-211?focusedCommentId=13473381&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13473381
Description
-------
Current limitations:
-Not tested on other than hadoop trunk
How to use:
use
-Dgiraph.useNetty=true -Dgiraph.authenticate=true
on your job's command line to use this feature.
How to compile and test:
mvn -Phadoop_trunk clean test
Works with the following test (after applying patch to trunk):
$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 -Dgiraph.authenticate=true -e 2 -s 10 -v -V 2 -w 2
This addresses bug GIRAPH-211.
https://issues.apache.org/jira/browse/GIRAPH-211
Diffs (updated)
-----
pom.xml 51944fa
src/main/java/org/apache/giraph/GiraphConfiguration.java 797ba6e
src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java 1849504
src/main/java/org/apache/giraph/comm/CommunicationsInterface.java d60f512
src/main/java/org/apache/giraph/comm/SecureRPCCommunications.java af9720e
src/main/java/org/apache/giraph/comm/WorkerClient.java c3ec4fe
src/main/java/org/apache/giraph/comm/netty/NettyClient.java 93f5671
src/main/java/org/apache/giraph/comm/netty/NettyServer.java 886d68b
src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java 5b71401
src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientServer.java 4710f3a
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/RequestEncoder.java 3174447
src/main/java/org/apache/giraph/comm/netty/handler/SaslClientHandler.java PRE-CREATION
src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java PRE-CREATION
src/main/java/org/apache/giraph/comm/requests/RequestType.java b412aa2
src/main/java/org/apache/giraph/comm/requests/SaslCompleteRequest.java PRE-CREATION
src/main/java/org/apache/giraph/comm/requests/SaslTokenMessageRequest.java PRE-CREATION
src/main/java/org/apache/giraph/graph/BspServiceWorker.java bb498ce
src/test/java/org/apache/giraph/comm/SaslConnectionTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/6609/diff/
Testing
-------
Tested only on Hadoop Trunk : needs testing on other Hadoops.
Thanks,
Eugene Koontz
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eugene Koontz <ek...@hiro-tan.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6609/
-----------------------------------------------------------
(Updated Sept. 27, 2012, 6:54 p.m.)
Review request for giraph.
Changes
-------
add missing value "true" for param "giraph.authenticate".
Description (updated)
-------
Current limitations:
-Not tested on other than hadoop trunk
How to use:
use
-Dgiraph.useNetty=true -Dgiraph.authenticate=true
on your job's command line to use this feature.
How to compile and test:
mvn -Phadoop_trunk clean test
Works with the following test (after applying patch to trunk):
$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 -Dgiraph.authenticate=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/WorkerClient.java c3ec4fe
src/main/java/org/apache/giraph/comm/netty/NettyClient.java e6a8508
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/Authorize.java PRE-CREATION
src/main/java/org/apache/giraph/comm/netty/handler/ResponseEncoder.java PRE-CREATION
src/main/java/org/apache/giraph/comm/netty/handler/SaslClientHandler.java PRE-CREATION
src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java PRE-CREATION
src/main/java/org/apache/giraph/comm/requests/RequestType.java 4fbf692
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/graph/BspServiceWorker.java d926d1c
src/main/java/org/apache/giraph/graph/GiraphJob.java 342be46
Diff: https://reviews.apache.org/r/6609/diff/
Testing
-------
Tested only on Hadoop Trunk : needs testing on other Hadoops.
Thanks,
Eugene Koontz
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eugene Koontz <ek...@hiro-tan.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6609/#review11989
-----------------------------------------------------------
src/main/java/org/apache/giraph/comm/netty/NettyClient.java
<https://reviews.apache.org/r/6609/#comment25558>
"restoring the *client* pipeline (note this differs compared the server pipeline case: compare with NettyServer.java)."
src/main/java/org/apache/giraph/comm/netty/NettyClient.java
<https://reviews.apache.org/r/6609/#comment25559>
Currently this waits until all channels finish authenticating. Consider not blocking on authenticating each channel - just initiate authentication for all channels and then wait for all channels to finish as a group: use a semaphore/latch that counts down to 0 as each channel authenticates.
- Eugene Koontz
On Sept. 20, 2012, 4:43 a.m., Eugene Koontz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6609/
> -----------------------------------------------------------
>
> (Updated Sept. 20, 2012, 4:43 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> Current limitations:
>
> -Not tested on other than hadoop trunk
>
> How to use:
>
> use
>
> -Dgiraph.useNetty=true -Dgiraph.authenticate
>
> on your job's command line to use this feature.
>
> How to compile and test:
>
> mvn -Phadoop_trunk clean test
>
> Works with the following test (after applying patch to trunk):
>
> $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 -Dgiraph.authenticate=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/WorkerClient.java c3ec4fe
> src/main/java/org/apache/giraph/comm/netty/NettyClient.java e6a8508
> 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/Authorize.java PRE-CREATION
> src/main/java/org/apache/giraph/comm/netty/handler/ResponseEncoder.java PRE-CREATION
> src/main/java/org/apache/giraph/comm/netty/handler/SaslClientHandler.java PRE-CREATION
> src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java PRE-CREATION
> src/main/java/org/apache/giraph/comm/requests/RequestType.java 4fbf692
> 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/graph/BspServiceWorker.java d926d1c
> src/main/java/org/apache/giraph/graph/GiraphJob.java 342be46
>
> Diff: https://reviews.apache.org/r/6609/diff/
>
>
> Testing
> -------
>
> Tested only on Hadoop Trunk : needs testing on other Hadoops.
>
>
> Thanks,
>
> Eugene Koontz
>
>
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eugene Koontz <ek...@hiro-tan.org>.
> On Sept. 27, 2012, 6:40 p.m., Eugene Koontz wrote:
> > src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java, line 1310
> > <https://reviews.apache.org/r/6609/diff/6/?file=158675#file158675line1310>
> >
> > Add:
> >
> > LOG.warn("You configured authentication at Giraph RPC level, but you are using Hadoop RPC to do Giraph RPC. Continuing with the assumption that your Hadoop is secure and not authenticating at Hadoop RPC level.");
Sorry, last part should read: "not authenticating at Giraph RPC level."
- Eugene
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6609/#review11988
-----------------------------------------------------------
On Sept. 20, 2012, 4:43 a.m., Eugene Koontz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6609/
> -----------------------------------------------------------
>
> (Updated Sept. 20, 2012, 4:43 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> Current limitations:
>
> -Not tested on other than hadoop trunk
>
> How to use:
>
> use
>
> -Dgiraph.useNetty=true -Dgiraph.authenticate
>
> on your job's command line to use this feature.
>
> How to compile and test:
>
> mvn -Phadoop_trunk clean test
>
> Works with the following test (after applying patch to trunk):
>
> $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 -Dgiraph.authenticate=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/WorkerClient.java c3ec4fe
> src/main/java/org/apache/giraph/comm/netty/NettyClient.java e6a8508
> 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/Authorize.java PRE-CREATION
> src/main/java/org/apache/giraph/comm/netty/handler/ResponseEncoder.java PRE-CREATION
> src/main/java/org/apache/giraph/comm/netty/handler/SaslClientHandler.java PRE-CREATION
> src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java PRE-CREATION
> src/main/java/org/apache/giraph/comm/requests/RequestType.java 4fbf692
> 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/graph/BspServiceWorker.java d926d1c
> src/main/java/org/apache/giraph/graph/GiraphJob.java 342be46
>
> Diff: https://reviews.apache.org/r/6609/diff/
>
>
> Testing
> -------
>
> Tested only on Hadoop Trunk : needs testing on other Hadoops.
>
>
> Thanks,
>
> Eugene Koontz
>
>
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eugene Koontz <ek...@hiro-tan.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6609/#review11988
-----------------------------------------------------------
src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java
<https://reviews.apache.org/r/6609/#comment25557>
Add:
LOG.warn("You configured authentication at Giraph RPC level, but you are using Hadoop RPC to do Giraph RPC. Continuing with the assumption that your Hadoop is secure and not authenticating at Hadoop RPC level.");
- Eugene Koontz
On Sept. 20, 2012, 4:43 a.m., Eugene Koontz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6609/
> -----------------------------------------------------------
>
> (Updated Sept. 20, 2012, 4:43 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> Current limitations:
>
> -Not tested on other than hadoop trunk
>
> How to use:
>
> use
>
> -Dgiraph.useNetty=true -Dgiraph.authenticate
>
> on your job's command line to use this feature.
>
> How to compile and test:
>
> mvn -Phadoop_trunk clean test
>
> Works with the following test (after applying patch to trunk):
>
> $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 -Dgiraph.authenticate=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/WorkerClient.java c3ec4fe
> src/main/java/org/apache/giraph/comm/netty/NettyClient.java e6a8508
> 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/Authorize.java PRE-CREATION
> src/main/java/org/apache/giraph/comm/netty/handler/ResponseEncoder.java PRE-CREATION
> src/main/java/org/apache/giraph/comm/netty/handler/SaslClientHandler.java PRE-CREATION
> src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java PRE-CREATION
> src/main/java/org/apache/giraph/comm/requests/RequestType.java 4fbf692
> 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/graph/BspServiceWorker.java d926d1c
> src/main/java/org/apache/giraph/graph/GiraphJob.java 342be46
>
> Diff: https://reviews.apache.org/r/6609/diff/
>
>
> Testing
> -------
>
> Tested only on Hadoop Trunk : needs testing on other Hadoops.
>
>
> Thanks,
>
> Eugene Koontz
>
>
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eugene Koontz <ek...@hiro-tan.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6609/
-----------------------------------------------------------
(Updated Sept. 20, 2012, 4:43 a.m.)
Review request for giraph.
Description (updated)
-------
Current limitations:
-Not tested on other than hadoop trunk
How to use:
use
-Dgiraph.useNetty=true -Dgiraph.authenticate
on your job's command line to use this feature.
How to compile and test:
mvn -Phadoop_trunk clean test
Works with the following test (after applying patch to trunk):
$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 -Dgiraph.authenticate=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/WorkerClient.java c3ec4fe
src/main/java/org/apache/giraph/comm/netty/NettyClient.java e6a8508
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/Authorize.java PRE-CREATION
src/main/java/org/apache/giraph/comm/netty/handler/ResponseEncoder.java PRE-CREATION
src/main/java/org/apache/giraph/comm/netty/handler/SaslClientHandler.java PRE-CREATION
src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java PRE-CREATION
src/main/java/org/apache/giraph/comm/requests/RequestType.java 4fbf692
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/graph/BspServiceWorker.java d926d1c
src/main/java/org/apache/giraph/graph/GiraphJob.java 342be46
Diff: https://reviews.apache.org/r/6609/diff/
Testing (updated)
-------
Tested only on Hadoop Trunk : needs testing on other Hadoops.
Thanks,
Eugene Koontz
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eugene Koontz <ek...@hiro-tan.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6609/
-----------------------------------------------------------
(Updated Sept. 20, 2012, 4:42 a.m.)
Review request for giraph.
Description (updated)
-------
Current limitations:
-Not tested on other than hadoop trunk
How to use:
use
-Dgiraph.useNetty=true -Dgiraph.authenticate
on your job's command line to use this feature.
How to compile and test:
mvn -Phadoop_trunk 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 -Dgiraph.authenticate=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/WorkerClient.java c3ec4fe
src/main/java/org/apache/giraph/comm/netty/NettyClient.java e6a8508
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/Authorize.java PRE-CREATION
src/main/java/org/apache/giraph/comm/netty/handler/ResponseEncoder.java PRE-CREATION
src/main/java/org/apache/giraph/comm/netty/handler/SaslClientHandler.java PRE-CREATION
src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java PRE-CREATION
src/main/java/org/apache/giraph/comm/requests/RequestType.java 4fbf692
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/graph/BspServiceWorker.java d926d1c
src/main/java/org/apache/giraph/graph/GiraphJob.java 342be46
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
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eugene Koontz <ek...@hiro-tan.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6609/
-----------------------------------------------------------
(Updated Sept. 20, 2012, 4:34 a.m.)
Review request for giraph.
Changes
-------
Matches https://issues.apache.org/jira/secure/attachment/12545806/GIRAPH-211.patch
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 (updated)
-----
src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java bc53b83
src/main/java/org/apache/giraph/comm/WorkerClient.java c3ec4fe
src/main/java/org/apache/giraph/comm/netty/NettyClient.java e6a8508
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/Authorize.java PRE-CREATION
src/main/java/org/apache/giraph/comm/netty/handler/ResponseEncoder.java PRE-CREATION
src/main/java/org/apache/giraph/comm/netty/handler/SaslClientHandler.java PRE-CREATION
src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java PRE-CREATION
src/main/java/org/apache/giraph/comm/requests/RequestType.java 4fbf692
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/graph/BspServiceWorker.java d926d1c
src/main/java/org/apache/giraph/graph/GiraphJob.java 342be46
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
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eugene Koontz <ek...@hiro-tan.org>.
-----------------------------------------------------------
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.
Changes
-------
Correct URL to patch in JIRA.
Description (updated)
-------
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
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eugene Koontz <ek...@hiro-tan.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6609/
-----------------------------------------------------------
(Updated Aug. 29, 2012, 11:46 p.m.)
Review request for giraph.
Changes
-------
merge with trunk: 55bfcb78 https://svn.apache.org/repos/asf/giraph/trunk@1378761 (GIRAPH-317)
Description
-------
Same patch as: https://issues.apache.org/jira/secure/attachment/12542964/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 (updated)
-----
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
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eugene Koontz <ek...@hiro-tan.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6609/
-----------------------------------------------------------
(Updated Aug. 29, 2012, 8:19 p.m.)
Review request for giraph.
Changes
-------
Updated to https://issues.apache.org/jira/secure/attachment/12542973/GIRAPH-211.patch
Description
-------
Same patch as: https://issues.apache.org/jira/secure/attachment/12542964/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 (updated)
-----
src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java bc53b83
src/main/java/org/apache/giraph/comm/MasterRequestServerHandler.java 16e163a
src/main/java/org/apache/giraph/comm/NettyClient.java 0353924
src/main/java/org/apache/giraph/comm/NettyServer.java 44a0043
src/main/java/org/apache/giraph/comm/NettyWorkerClient.java e5822e1
src/main/java/org/apache/giraph/comm/NettyWorkerClientServer.java 139e6f0
src/main/java/org/apache/giraph/comm/NullReply.java PRE-CREATION
src/main/java/org/apache/giraph/comm/RequestServerHandler.java eae3f87
src/main/java/org/apache/giraph/comm/RequestType.java e363ad8
src/main/java/org/apache/giraph/comm/ResponseClientHandler.java 3adee8d
src/main/java/org/apache/giraph/comm/ResponseEncoder.java PRE-CREATION
src/main/java/org/apache/giraph/comm/SaslComplete.java PRE-CREATION
src/main/java/org/apache/giraph/comm/SaslNettyClient.java PRE-CREATION
src/main/java/org/apache/giraph/comm/SaslNettyServer.java PRE-CREATION
src/main/java/org/apache/giraph/comm/SaslTokenMessage.java PRE-CREATION
src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java 4944894
src/main/java/org/apache/giraph/comm/SendPartitionMutationsRequest.java c39ba60
src/main/java/org/apache/giraph/comm/SendVertexRequest.java 014043d
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/WorkerRequestServerHandler.java ee4e286
src/main/java/org/apache/giraph/comm/WritableRequest.java 931feb7
src/main/java/org/apache/giraph/comm/messages/SendPartitionCurrentMessagesRequest.java b10e6e9
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
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eugene Koontz <ek...@hiro-tan.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6609/
-----------------------------------------------------------
(Updated Aug. 29, 2012, 7 p.m.)
Review request for giraph.
Changes
-------
Updated to https://issues.apache.org/jira/secure/attachment/12542964/GIRAPH-211.patch
Description (updated)
-------
Same patch as: https://issues.apache.org/jira/secure/attachment/12542964/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 (updated)
-----
src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java bc53b83
src/main/java/org/apache/giraph/comm/MasterRequestServerHandler.java 16e163a
src/main/java/org/apache/giraph/comm/NettyClient.java 0353924
src/main/java/org/apache/giraph/comm/NettyServer.java 44a0043
src/main/java/org/apache/giraph/comm/NettyWorkerClient.java e5822e1
src/main/java/org/apache/giraph/comm/NettyWorkerClientServer.java 139e6f0
src/main/java/org/apache/giraph/comm/NullReply.java PRE-CREATION
src/main/java/org/apache/giraph/comm/RequestServerHandler.java eae3f87
src/main/java/org/apache/giraph/comm/RequestType.java e363ad8
src/main/java/org/apache/giraph/comm/ResponseClientHandler.java 3adee8d
src/main/java/org/apache/giraph/comm/ResponseEncoder.java PRE-CREATION
src/main/java/org/apache/giraph/comm/SaslComplete.java PRE-CREATION
src/main/java/org/apache/giraph/comm/SaslNettyClient.java PRE-CREATION
src/main/java/org/apache/giraph/comm/SaslNettyServer.java PRE-CREATION
src/main/java/org/apache/giraph/comm/SaslTokenMessage.java PRE-CREATION
src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java 4944894
src/main/java/org/apache/giraph/comm/SendPartitionMutationsRequest.java c39ba60
src/main/java/org/apache/giraph/comm/SendVertexRequest.java 014043d
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/WorkerRequestServerHandler.java ee4e286
src/main/java/org/apache/giraph/comm/WritableRequest.java 931feb7
src/main/java/org/apache/giraph/comm/messages/SendPartitionCurrentMessagesRequest.java b10e6e9
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
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eugene Koontz <ek...@hiro-tan.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6609/
-----------------------------------------------------------
(Updated Aug. 29, 2012, 6:46 p.m.)
Review request for giraph.
Changes
-------
Updated to correspond to https://issues.apache.org/jira/secure/attachment/12542963/GIRAPH-211.patch
Description (updated)
-------
Same patch as: https://issues.apache.org/jira/secure/attachment/12542963/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/MasterRequestServerHandler.java 16e163a
src/main/java/org/apache/giraph/comm/NettyClient.java 0353924
src/main/java/org/apache/giraph/comm/NettyServer.java 44a0043
src/main/java/org/apache/giraph/comm/NettyWorkerClient.java e5822e1
src/main/java/org/apache/giraph/comm/NettyWorkerClientServer.java 139e6f0
src/main/java/org/apache/giraph/comm/RequestServerHandler.java eae3f87
src/main/java/org/apache/giraph/comm/RequestType.java e363ad8
src/main/java/org/apache/giraph/comm/ResponseClientHandler.java 3adee8d
src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java 4944894
src/main/java/org/apache/giraph/comm/SendPartitionMutationsRequest.java c39ba60
src/main/java/org/apache/giraph/comm/SendVertexRequest.java 014043d
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/WorkerRequestServerHandler.java ee4e286
src/main/java/org/apache/giraph/comm/WritableRequest.java 931feb7
src/main/java/org/apache/giraph/comm/messages/SendPartitionCurrentMessagesRequest.java b10e6e9
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
Re: Review Request: GIRAPH-211: Add secure authentication to Netty IPC
Posted by Eugene Koontz <ek...@hiro-tan.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6609/
-----------------------------------------------------------
(Updated Aug. 29, 2012, 6:44 p.m.)
Review request for giraph.
Changes
-------
I manually fixed some conflicts recently, so I'm gradually making this patch more closely fit to trunk.
Fortunately, this patch is half the size of the last patch.
Improvements:
-Removes usage of RequestRegistry (that was removed in GIRAPH-313)
Limitations:
Uses preGIRAPH-313 workerThreadPool rather than requestServerHandlerFactory
-Needs tests
Description
-------
Same patch as https://issues.apache.org/jira/secure/attachment/12539928/GIRAPH-211.patch
Current limitations:
-Authorization is not done: that is, clients are authenticated but there are no restrictions on their ability to do RPC on the servers.
-Clients should wait for authentication before trying to do RPC - once authorization (see above) is done, they might encounter a race where they try to do RPCs without yet being authenticated.
-Not tested on other than hadoop 2.0.1-SNAPSHOT
-Only works if we disable client-side channel-pooling (GIRAPH-289) and local short-circuiting of RPCs (GIRAPH-262) - these should be configurable but currently, I hard-wired both to be disabled.
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 (updated)
-----
src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java bc53b83
src/main/java/org/apache/giraph/comm/MasterRequestServerHandler.java 16e163a
src/main/java/org/apache/giraph/comm/NettyClient.java 0353924
src/main/java/org/apache/giraph/comm/NettyServer.java 44a0043
src/main/java/org/apache/giraph/comm/NettyWorkerClient.java e5822e1
src/main/java/org/apache/giraph/comm/NettyWorkerClientServer.java 139e6f0
src/main/java/org/apache/giraph/comm/RequestServerHandler.java eae3f87
src/main/java/org/apache/giraph/comm/RequestType.java e363ad8
src/main/java/org/apache/giraph/comm/ResponseClientHandler.java 3adee8d
src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java 4944894
src/main/java/org/apache/giraph/comm/SendPartitionMutationsRequest.java c39ba60
src/main/java/org/apache/giraph/comm/SendVertexRequest.java 014043d
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/WorkerRequestServerHandler.java ee4e286
src/main/java/org/apache/giraph/comm/WritableRequest.java 931feb7
src/main/java/org/apache/giraph/comm/messages/SendPartitionCurrentMessagesRequest.java b10e6e9
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