bblfsh / sdk

Babelfish driver SDK
GNU General Public License v3.0
23 stars 27 forks source link

Pool of native drivers (prototype) #443

Closed kuba-- closed 4 years ago

kuba-- commented 4 years ago

Signed-off-by: kuba-- kuba@sourced.tech

This is the first draft/prototype which implements a pool of native drivers (parsers). Instead of having just one native driver we handle a pool of concurrent drivers, so for each grpc.Parse request we can pull the driver, write request and put them back to the pool. The pool of drivers was implemented as buffered channel.

This PR closes https://github.com/bblfsh/bblfshd/issues/321 and it's a part of epic: https://github.com/bblfsh/bblfshd/issues/313

Some design details you can find in doc: https://docs.google.com/document/d/1671ApmxFCOmerVKloVC6Q6j8BsJr8xZHbhuE40ZK-as/edit?pli=1#


This change is Reviewable

ncordon commented 4 years ago

driver/impl.go, line 43 at r2 (raw file):


// Start gets each driver from the channel, starts the process
// and put it back to the same buffer channel.

Feel free to ignore this nitpick: puts

ncordon commented 4 years ago

driver/impl.go, line 59 at r2 (raw file):


// Close tries to close all idle drivers.
// If any of drivers fail on Close, the function wraps an error (err) and move on.

moves

ncordon commented 4 years ago

driver/impl.go, line 47 at r3 (raw file):

// the function tries to close all running drivers and return an error.
func (d *driverImpl) Start() error {
  for i := 0; i < len(d.ch); i++ {

Does this start all processes even if we are not going to use them? I mean if I have 8 processors, am I going to start 8 processes even if I am just going to parse a file?

ncordon commented 4 years ago

driver/impl.go, line 47 at r3 (raw file):

Previously, dennwc (Denys Smirnov) wrote…
@ncordon Also note that we do this to simplify things. In K8S we can disable the scaling here (set it to 1 driver) and let K8S scale whole drivers, not native ones.

:ok_hand: Thanks for the clarification