neofs-node icon indicating copy to clipboard operation
neofs-node copied to clipboard

Refactor IR

Open notimetoname opened this issue 1 year ago • 2 comments

Some questions:

  1. Why does it have workers, starters and runners? Also, all of them are anonymous, it makes it difficult to understand what "services" (well, just func ()) are running/will be running.
  2. Why does it read config everywhere, everywhere, everywhere, etc it wants? SN has been using a separate place for config reading for years already. After config is parsed it passes (and modifies its types if need be) in every component that requires it.
  3. Its constructor is 650+ lines long. It does many things (converting types, creating new types, reading configs, making some RPC, logging, filling DB, deciding the full IR work mode in a huge if ... else, etc). I mean that is not bad that everything is happening at the New step, I mean that most of that is done literally inside that func.
  4. https://github.com/nspcc-dev/neofs-node/issues/2309 will require changes that should drop processors, listeners all related things. #2309 says "Each service can do Receive* for things it needs..." but IR has only two services in fact: timers (not services TBH, they only can accept Tick calls and be Reseted), and a Listener that should be dropped. It also makes anything related to https://github.com/nspcc-dev/neofs-node/issues/1337 hard to implement easily.

notimetoname avatar Jun 28 '23 15:06 notimetoname

The way I see it working is:

  • IR main reads config and keeps it internally forever
  • it creates connections
  • it passes them to innerring.New()
  • which can do whatever it needs to initialialize (read various data) and set up event listeners
  • it works until some connection breaks
  • when it breaks IR stops completely (including additional goroutines if any)
  • main kicks in, reconnects to RPC
  • and restarts IR again

The rationale is rather simple: IR state is completely stored on blockchain, it's a service that doesn't differ much from Oracle or Notary services, so it MUST NOT deal with connections internally at all, it should take them as a given. It also shouldn't care about configuration files, these are cmd things by definition.

roman-khimov avatar Jun 29 '23 17:06 roman-khimov

if Inner Ring is based on and works around blockchain, then in my mind it should be constructed from type Blockchain interface providing all IR-expected stuff. From IR pov, Blockchain wont deal with connections (no endpoint, conn parameters).

cthulhu-rider avatar Jul 03 '23 04:07 cthulhu-rider