search

hyperledger-labs / Scorex

Scorex 2.0 Core
Apache License 2.0
543 stars 115 forks source link

Remember time of last seen message #374

Closed kushti closed 3 years ago

kushti commented 3 years ago

Do this on top of PR #373 :

1) Like #373 is using message got from a peer to drop inactive connection (in NetworkController), introduce global variable in NetworkController which is storing when we got last message from the network

2) Introduce a new API method /peers/status (PeersApiRoute) which is returning something like {"lastIncomingMessage":XXX, "currentNetworkTime":XXX}

knizhnik commented 3 years ago

I have several questions:

  1. So lastSeen - is timestamp of receiving last message from the particular peer. Why do we need to add some variable to NetworkController? Why can't we just calculate min(lastSeen) for all connections?
  2. Do we really need to add new /peers/status method? May be just add lastIncomingMessage to NodeInfo, returned by /info method? If /peers/status method is really needed, what should it return, just "currentNetworkTime" == NodeInfo.currentTime and "lastIncomingMessage"?
kushti commented 3 years ago

@knizhnik My bad likely, it was better to start with motivation. Which is about the need to recognize whether connectivity globally (with all the peers) is lost or not. If it lost, then we can consider our syncing status differently, show warning in a UI etc.

Thus this variable is global for all the peers.

/info is not in Scorex, it is Scorex-specific. In Ergo, we can enrich /info for sure.

knizhnik commented 3 years ago

Sorry, I do not understand your answers. My first questions was why do we need to add some global variable to NetworkController insteadof just calculating max(lastSeen) ?

Also I do not understand "/info is not in Scorex, it is Scorex-specific". All API methods are declared in openapi.yaml aren't them? No AI methods are defined in Scorex, aren't them? So, as far as I understand this task requires patching both "scorex" and "ergo" projects.

knizhnik commented 3 years ago

I have created PR https://github.com/ScorexFoundation/Scorex/pull/376 Not sure that it is enough and whether openapi.yaml in ergo should also be changed.

kushti commented 3 years ago

Done in #376