New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

node types are confusing #21

Open
matthewdarwin opened this Issue Jul 16, 2018 · 16 comments

Comments

Projects
None yet
@matthewdarwin
Copy link

matthewdarwin commented Jul 16, 2018

We have 4 node types. It is not clear what is the definition for each. This is what seems to be in use today:

producer: for producer nodes. the actual end points should not be published
query: for nodes you can query with http or https
seed: for nodes you can connect on p2p or bnet
full: both query and seed

However, there are some thoughts that 'query' is useless and the definition i put for query should be used for 'full'. Thoughts?

@ScottSallinen

This comment has been minimized.

Copy link

ScottSallinen commented Jul 16, 2018

Query doesn't make any sense to me. I have no idea what it would be for.
In steem, we have had 3 clear cut definitions for years, and I'd suggest we use them here too. Here's what I'd suggest, following how nodes actually should operate:

  1. producers: No endpoints. Location optional. Discussing your producer in general is optional.
  2. seed: Connect via p2p or bnet in order to retrieve blocks and connect to the global network. Location encouraged / required.
  3. full: Offers HTTP(S) endpoints for API calls. Location encouraged / required. p2p/bnet endpoints actively discouraged.

For 1), hidden producer locations should be acceptable, if not encouraged. For 2), seed nodes need not provide anything beyond the ability to link up to the p2p network -- but should provide a large amount of connections and have geographic diversity. For 3), Full nodes should not provide p2p/bnet connections. You should not mix a seed node and a full node. Full nodes are strictly responsible for servicing API requests, and should not be responsible for bootstrapping new nodes entering the network. They should not be bottlenecked by hundreds of clients trying to p2p/bnet connect, their bandwidth and client connections should be reserved for API requests.

@DenisCarriere

This comment has been minimized.

Copy link
Contributor

DenisCarriere commented Jul 16, 2018

@ScottSallinen 👍 I like the 3 classification approach.

I also agree that your HTTP(s) API should be solely responsible for servicing API requests and not be slowed down by P2P connections.

I support having the following node classification:

  • producer
  • seed
  • full

BP Info Standard would deprecate/drop the query classification

@ScottSallinen

This comment has been minimized.

Copy link

ScottSallinen commented Jul 16, 2018

Furthermore, a BP should have at minimum one seed and one full. Preferably offering more as they increase in rank.
I don't think it's acceptable that high ranking BPs shove all their public infrastructure into one node and can call it a day. Their one node is going to get bottlenecked pretty quick.

@craigmurray100

This comment has been minimized.

Copy link

craigmurray100 commented Jul 16, 2018

Agree with the 3 listed by ScottSallinen

matthewdarwin added a commit to EOS-Nation/bpvalidate that referenced this issue Jul 16, 2018

@bensig

This comment has been minimized.

Copy link

bensig commented Jul 17, 2018

Could someone explain "full" as a node type? I would probably lose that one since the term "full" means "everything" - I would define the types as:
producer - no published endpoint
seed - p2p and/or bnet
query - only api

@ScottSallinen

This comment has been minimized.

Copy link

ScottSallinen commented Jul 17, 2018

"Full" was terminology from Steem, and it referred to basically "providing all API plugins".
I'm fine with scrapping that term and moving over to something like "API" node, as that makes more sense to me anyways.

There still needs to be a standardization of a minimum APIs & action histories that an api/full node should provide, as it will be impossible to filter-on = * soon.
Some info on that here: https://steemit.com/eos/@greymass/consistency-in-configuration-of-public-eos-full-nodes

@GMory

This comment has been minimized.

Copy link

GMory commented Jul 17, 2018

Agreed with @ScottSallinen. The classifications (Producer, Seed, Full) cover the full scope of the node roles and removes the extraneous classification. Plus I think most BPs infrastructures are rolling with just three types of nodes which are conforming to the proposed definitions of the Producer, Seed, Full.

@eluzgin

This comment has been minimized.

Copy link

eluzgin commented Jul 19, 2018

I would argue for the following classification:
producer - should be hidden (only share geographical location).
seed - only P2P endpoint
full - Full API endpoint (and seed node currently?)

In reality we currently have commonly used 2 types of nodes: producers and full, which also serve as seed nodes. We probably should encourage separation between seed and full nodes?
If that's the case - we can add those requirements into bp-info-standard.

@n8d

This comment has been minimized.

Copy link
Contributor

n8d commented Jul 24, 2018

I agree "query" is confusing and should be removed.

Internally we (Aloha EOS) have been using:

  • producer (no endpoints)
  • peer (p2p and/or bnet)
  • api (http and/or https)

Which I think are a little more clear. However if "seed" and "full" are the more popular terms that sounds fine.

matthewdarwin added a commit to EOS-Nation/bpvalidate that referenced this issue Aug 6, 2018

@matthewdarwin

This comment has been minimized.

Copy link

matthewdarwin commented Aug 6, 2018

validate.eosn.io now does the following checks (pseudo code)

    if (! $node_type) {
      $self->add_message(detail => "node_type is not provided, set it to one of the following values ['producer', 'full', 'query', 'seed']");
    } elsif ($node_type eq 'producer') {
      if ($found_something) {
        $self->add_message(detail => 'endpoints provided (producer should be private)');
      }
    } elsif ($node_type eq 'seed') {
      if (! $peer_endpoint && ! $bnet_endpoint) {
        $self->add_message(detail => 'no valid peer endpoints provided');
      }
      if ($api_endpoint || $ssl_endpoint) {
        $self->add_message(detail => 'extranious API endpoints provided');
      }
    } elsif ($node_type eq 'query') {
      $self->add_message(detail => 'use node_type=query is deprecated; use node_type=full instead')
    } elsif ($node_type eq 'full') {
      if ($peer_endpoint || $bnet_endpoint) {
        $self->add_message(detail => 'extranious peer endpoints provided');
      }
      if (! $api_endpoint && ! $ssl_endpoint) {
        $self->add_message(detail => 'no valid API endpoints provided');
      }
    } else {
      $self->add_message(detail => "node_type is not valid, set it to one of the following values ['producer', 'full', 'query', 'seed']");
      if (! $found_something) {
        $self->add_message(detail => 'no valid endpoints provided (useless section)');
      }
    }
@igorls

This comment has been minimized.

Copy link
Collaborator

igorls commented Aug 7, 2018

I don't think full node is a good name if we don't allow p2p, we should then have only ["producer","seed","api"]

@DenisCarriere

This comment has been minimized.

Copy link
Contributor

DenisCarriere commented Aug 7, 2018

Maybe add a 4th type?

["producer","seed","api","full"]

  • producer = Hidden Block Producer
  • seed = Only P2P (no exposed API)
  • api = Only API (no seeding connections)
  • full = Both API & P2P
@samspk

This comment has been minimized.

Copy link

samspk commented Aug 7, 2018

What is not matching here?
image

@matthewdarwin

This comment has been minimized.

Copy link

matthewdarwin commented Aug 8, 2018

@igorls should make a decision one way another and resolve this issue.

@lukestokes

This comment has been minimized.

Copy link

lukestokes commented Aug 23, 2018

We're currently using "full" but getting a note pointing to this thread:

extranious peer endpoints provided, field=<node[0]>, having node_type=

Can we get some clarity on the expectations here? Right now a "full" node is both an HTTP/s API and a p2p seed node.

Seems odd to throw a warning note if this issue is unresolved. As the original message states "we have four types" so I don't see why using "full" should get a warning?

@bensig

This comment has been minimized.

Copy link

bensig commented Aug 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment