beacon/light/request: general request framework (WIP) #28656
Conversation
|
|
||
| type RequestServer interface { | ||
| Subscribe(eventCallback func(event Event)) | ||
| CanSendRequest(request interface{}) (bool, float32) |
There was a problem hiding this comment.
Since Go 1.18, you can use any instead of interface{}
| type RequestServer interface { | ||
| Subscribe(eventCallback func(event Event)) | ||
| CanSendRequest(request interface{}) (bool, float32) | ||
| SendRequest(request interface{}) (reqId interface{}) |
There was a problem hiding this comment.
Do you think it would be possible to define a concrete type for request IDs here? Why keep it so abstract?
I don't understand this. The server for a light client is, at the moment, an http endpoint. Why not just
And done. What do we need a general purpose request framework for, when all we do is query a known api? |
I think this PR will make more sense in the context of the subsequent one :) Sure, the resend/timeout logic could be implemented as a multiplexer for the API, but I think the sync logic will be much cleaner with this event driven approach. See #26874 (comment) |
In general, it's nicer API to use a blocking api, instead of having to work with callbacks. I mean, we (actually @karalabe) did rewrite large parts of the p2p request-handling on snap/eth just to "feel" like it was a blocking API, to hide the facts that internally it was just actually an async event handling. |
| func (s *Scheduler) syncLoop() { | ||
| for { | ||
| s.lock.Lock() | ||
| s.processModules() |
There was a problem hiding this comment.
the processModules is holding s.lock. If any of these generic modules try to interact with the scheduler, e..g registering a new server, this will deadlock, no?
I'll think about it. Maybe I am trying to do too much abstraction at an early stage again :) A separate goroutine for each sync process certainly doesn't hurt anyone and I am also starting to like the API multiplexer idea. The logic in this PR is fairly easy to restructure. Also, it can be done independently from the other parts so we can finish a single server version of |
|
Closed in favor of #28906 |
This PR implements a general-purpose framework for sync mechanisms requesting data from multiple servers. It is intended to be used primarily by the beacon chain light client but might be used for other purposes too as it does nothing specific to the beacon chain. It implements timeouts, very simple rate limits and server selection, basically abstracting away the multiple server logic from the actual sync processes.