Derecho-Project / derecho

The main code repository for the Derecho project.
BSD 3-Clause "New" or "Revised" License
187 stars 47 forks source link

enable the feature discussed in #227. #228

Closed songweijia closed 2 years ago

etremel commented 2 years ago

It looks like this should work, as long as RPC functions aren't re-entrant and user classes understand that the "RPC caller ID" is a cache of the last observed caller ID (i.e. calling it outside an RPC method will give an unreliable answer). I have one API-related question: Should the author of an RPC-callable method really need to know about RPCManager in order to find out the ID of the node calling the method? All of our other "user-facing" functions are in Group or GroupReference, and RPCManager is more of an "internal" class, so maybe it would be better to put a get_rpc_caller_id function in Group.

songweijia commented 2 years ago

I asked myself should I expose this API through _Group(the one an RPC function has access to) or not too. The fact is the RPCManager is actually visible to the derecho user-provided RPC functions. I picked the simplest possible way to implement it. However, we can add an inline static member method _Group::get_rpc_caller_id() to hide the RPCManager. What do you think?

Btw, please note that even RPC functions are re-entrant, this works because the static member is thread-local. We have a p2p worker thread and the predicate thread will call the RPC functions and it works fine in my tests.

etremel commented 2 years ago

Oh, I didn't realize you were waiting on my response here, but yes I think you should add a get_rpc_caller_id in _Group that calls the one in RPCManager. And it's good to hear that you've tested the case of a P2P function calling an ordered RPC function; it sounds like this will work then.

songweijia commented 2 years ago

I added _Group::get_rpc_caller_id(). I didn't make it a static function. Instead, I wrapped the static RPCManager::get_rpc_caller_id() in that API because the RPC handlers only have easy access to the _Group instance instead of its type.