Commit graph

241 commits

Author SHA1 Message Date
Roman Khimov
eeeb0f6f0e core: accept two-side channels for sub/unsub, read on unsub
Blockchain's notificationDispatcher sends events to channels and these
channels must be read from. Unfortunately, regular service shutdown procedure
does unsubscription first (outside of the read loop) and only then drains the
channel. While it waits for unsubscription request to be accepted
notificationDispatcher can try pushing more data into the same channel which
will lead to a deadlock. Reading in the same method solves this, any number of
events can be pushed until unsub channel accepts the data.
2022-08-19 22:08:40 +03:00
Roman Khimov
dea75a4211 network: wait for the relayer thread to finish on shutdown
Unsubscribe and drain first, then return from the Shutdown method. It's
important wrt to subsequent chain shutdown process (normally it's closed right
after the network server).
2022-08-19 22:08:40 +03:00
Anna Shaleva
916f2293b8 *: apply go 1.19 formatter heuristics
And make manual corrections where needed. See the "Common mistakes
and pitfalls" section of https://tip.golang.org/doc/comment.
2022-08-09 15:37:52 +03:00
Roman Khimov
9b0ea2c21b network/consensus: always process dBFT messages as high priority
Move category definition from consensus to payload, consensus service is the
one of its kind (HP), so network.Server can be adjusted accordingly.
2022-08-02 13:07:18 +03:00
Roman Khimov
94a8784dcb network: allow to drop services and solve concurrency issues
Now that services can come and go we need to protect all of the associated
fields and allow to deregister them.
2022-08-02 13:05:39 +03:00
Anna Shaleva
1ae601787d network: allow to handle GetMPTData with KeepOnlyLatestState on
And adjust documentation along the way.
2022-07-14 14:33:20 +03:00
Roman Khimov
3fbc1331aa
Merge pull request #2582 from nspcc-dev/fix-server-sync
network: adjust the way (*Server).IsInSync() works
2022-07-05 12:28:20 +03:00
Anna Shaleva
0835581fa9 network: adjust the way (*Server).IsInSync() works
Always return true if sync was reached once. Fix #2564.
2022-07-05 12:20:31 +03:00
Roman Khimov
3e2eda6752 *: add some comments to service Start/Shutdown methods 2022-07-04 23:03:50 +03:00
Anna Shaleva
8ab422da66 *: properly unsubscribe from Blockchain events 2022-06-28 19:09:25 +03:00
Elizaveta Chichindaeva
28908aa3cf [#2442] English Check
Signed-off-by: Elizaveta Chichindaeva <elizaveta@nspcc.ru>
2022-05-04 19:48:27 +03:00
Roman Khimov
2593bb0535 network: extend Service with Name, use it to distinguish services 2022-04-26 00:31:48 +03:00
Roman Khimov
e621f746a7 config/core: allow to change the number of validators
Fixes #2320.
2022-01-31 23:14:38 +03:00
Roman Khimov
60d6fa1125 network: keep a copy of the config inside of Server
Avoid copying the configuration again and again, make things a bit more
efficient.
2022-01-24 18:43:01 +03:00
Roman Khimov
89d754da6f network: don't request blocks we already have in the queue
Fixes #2258.
2022-01-18 00:04:41 +03:00
Roman Khimov
bc6d6e58bc network: always pass transactions to consensus process
Consensus can require conflicting transactions and it can require more
transactions than mempool can fit, all of this should work. Transactions will
be checked anyway using its secondary mempool. See the scenario from #668.
2022-01-14 20:08:40 +03:00
Roman Khimov
746644a4eb network: decouple it from blockchainer.Blockchainer
We don't need all of it.
2022-01-14 19:57:16 +03:00
Roman Khimov
bf1604454c blockchainer/network: move StateSync interface to the user
Only network package cares about it.
2022-01-14 19:57:14 +03:00
Roman Khimov
af87cb082f network: decouple Server from the notary service 2022-01-14 19:55:53 +03:00
Roman Khimov
508d36f698 network: drop consensus dependency 2022-01-14 19:55:53 +03:00
Roman Khimov
66aafd868b network: unplug stateroot service from the Server
Notice that it makes the node accept Extensible payloads with any category
which is the same way C# node works. We're trusting Extensible senders,
improper payloads are harmless until they DoS the network, but we have some
protections against that too (and spamming with proper category doesn't differ
a lot).
2022-01-14 19:55:50 +03:00
Roman Khimov
0ad3ea5944 network/cli: move Oracle service instantiation out of the network 2022-01-14 19:53:45 +03:00
Roman Khimov
5dd4db2c02 network/services: unify service lifecycle management
Run with Start, Stop with Shutdown, make behavior uniform.
2022-01-14 19:53:45 +03:00
Roman Khimov
c942402957 blockchainer: drop Policer interface
We never use it as a proper interface, so it makes no sense keeping it this
way.
2022-01-12 00:58:03 +03:00
Roman Khimov
2eeec73770 network: don't panic if there is no reason for disconnect
Although error should always be there, we shouldn't fail like this if it's not:
    | panic: runtime error: invalid memory address or nil pointer dereference
    | [signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0xc8884c]
    |
    | goroutine 113 [running]:
    | github.com/nspcc-dev/neo-go/pkg/network.(*Server).run(0xc000150580)
    |         github.com/nspcc-dev/neo-go/pkg/network/server.go:396 +0x7ac
    | github.com/nspcc-dev/neo-go/pkg/network.(*Server).Start(0xc000150580, 0x0)
    |         github.com/nspcc-dev/neo-go/pkg/network/server.go:294 +0x3fb
    | created by github.com/nspcc-dev/neo-go/cli/server.startServer
    |         github.com/nspcc-dev/neo-go/cli/server/server.go:344 +0x56f
2021-11-01 12:19:00 +03:00
AnnaShaleva
2d196b3f35 rpc: refactor calculatenetworkfee handler
Use (Blockchainer).VerifyWitness() to calculate network fee for
contract-based witnesses.
2021-10-25 19:07:25 +03:00
Evgeniy Stratonikov
4dd3a0d503 network: request headers in parallel, fix #2158
Do this similarly to how blocks are requested.
See also 4aa1a37.

Signed-off-by: Evgeniy Stratonikov <evgeniy@nspcc.ru>
2021-10-06 15:25:54 +03:00
Anna Shaleva
0fa48691f7 network: do not duplicate MPT nodes in GetMPTNodes response
Also tests are added.
2021-09-08 14:25:54 +03:00
Anna Shaleva
3b7807e897 network: request unknown MPT nodes
In this commit:

1. Request unknown MPT nodes from peers. Note, that StateSync module itself
shouldn't be responsible for nodes requests, that's a server duty.
2. Do not request the same node twice, check if it is in storage
already. If so, then the only thing remaining is to update refcounter.
2021-09-07 19:43:27 +03:00
Anna Shaleva
d67ff30704 core: implement statesync module
And support GetMPTData and MPTData P2P commands.
2021-09-07 19:43:27 +03:00
Roman Khimov
5aff82aef4
Merge pull request #2119 from nspcc-dev/states-exchange/insole
core, network: prepare basis for Insole module
2021-08-12 10:35:02 +03:00
Anna Shaleva
6ca7983be8 network: fix typo in error message 2021-08-10 11:00:39 +03:00
Roman Khimov
7bb82f1f99 network: merge two loops in iteratePeersWithSendMsg, send to 2/3
Refactor code and be fine with sending to just 2/3 of proper peers. Previously
it was an edge case, but it can be a normal thing to do also as broadcasting
to everyone is obviously too expensive and excessive (hi, #608).

Baseline (four node, 10 workers):

RPS    8180.760 8137.822 7858.358 7820.011 8051.076 ≈ 8010   ± 2.04%
TPS    7819.831 7521.172 7519.023 7242.965 7426.000 ≈ 7506   ± 2.78%
CPU %    41.983   38.775   40.606   39.375   35.537 ≈   39.3 ± 6.15%
Mem MB 2947.189 2743.658 2896.688 2813.276 2863.108 ≈ 2853   ± 2.74%

Patched:

RPS    9714.567 9676.102 9358.609 9371.408 9301.372 ≈ 9484   ±  2.05% ↑ 18.40%
TPS    8809.796 8796.854 8534.754 8661.158 8426.162 ≈ 8646   ±  1.92% ↑ 15.19%
CPU %    44.980   45.018   33.640   29.645   43.830 ≈   39.4 ± 18.41% ↑  0.25%
Mem MB 2989.078 2976.577 2306.185 2351.929 2910.479 ≈ 2707   ± 12.80% ↓  5.12%

There is a nuance with this patch however. While typically it works the way
outlined above, sometimes it works like this:

RPS ≈ 6734.368
TPS ≈ 6299.332
CPU ≈ 25.552%
Mem ≈ 2706.046MB

And that's because the log looks like this:

DeltaTime, TransactionsCount, TPS
5014, 44212, 8817.710
5163, 49690, 9624.249
5166, 49523, 9586.334
5189, 49693, 9576.604
5198, 49339, 9491.920
5147, 49559, 9628.716
5192, 49680, 9568.567
5163, 49750, 9635.871
5183, 49189, 9490.450
5159, 49653, 9624.540
5167, 47945, 9279.079
5179, 2051, 396.022
5015, 4, 0.798
5004, 0, 0.000
5003, 0, 0.000
5003, 0, 0.000
5003, 0, 0.000
5003, 0, 0.000
5004, 0, 0.000
5003, 2925, 584.649
5040, 49099, 9741.865
5161, 49718, 9633.404
5170, 49228, 9521.857
5179, 49773, 9610.543
5167, 47253, 9145.152
5202, 49788, 9570.934
5177, 47704, 9214.603
5209, 46610, 8947.975
5249, 49156, 9364.831
5163, 18284, 3541.352
5072, 174, 34.306

On a network with 4 CNs and 1 RPC node there is 1/256 probability that a block
won't be broadcasted to RPC node, so it won't see it until ping timeout kicks
in. While it doesn't see a block it can't accept new incoming transactions so
the bench gets stuck basically. To me that's an acceptable trade-off because
normal networks are much larger than that and the effect of this patch is way
more important there, but still that's what we have and we need to take into
account.
2021-08-06 21:10:34 +03:00
Roman Khimov
966a16e80e network: keep track of dead peers in iteratePeersWithSendMsg()
send() can return errStateMismatch, errGone and errBusy. errGone means the
peer is dead and it won't ever be active again, it doesn't make sense retrying
sends to it. errStateMismatch is technically "not yet ready", but we can't
wait for it either, no one knows how much will it take to complete
handshake. So only errBusy means we can retry.

So keep track of dead peers and adjust tries counting appropriately.
2021-08-06 21:10:34 +03:00
Roman Khimov
80f3ec2312 network: move peer filtering to getPeers()
It doesn't change much, we can't magically get more valid peers and if some
die while we're iterating we'd detect that by an error returned from send().
2021-08-06 21:10:34 +03:00
Roman Khimov
de6f4987f6 network: microoptimize iteratePeersWithSendMsg()
Now that s.getPeers() returns a slice we can use slice for `success` too, maps
are more expensive.
2021-08-06 21:10:34 +03:00
Roman Khimov
d51db20405 network: randomize peer iteration order
While iterating over map in getPeers() is non-deterministic it's not really
random enough for our purposes (usually maps have 2-3 paths through them), we
need to fill our peers queues more uniformly.

Believe it or not, but it does affect performance metrics, baseline (four
nodes, 10 workers):

RPS ≈  7791.675 7996.559 7834.504 7746.705 7891.614 ≈ 7852   ±  1.10%
TPS ≈  7241.497 7711.765 7520.211 7425.890 7334.443 ≈ 7447   ±  2.17%
CPU %    29.853   39.936   39.945   36.371   39.999 ≈   37.2 ± 10.57%
Mem MB 2749.635 2791.609 2828.610 2910.431 2863.344 ≈ 2829   ±  1.97%

Patched:

RPS    8180.760 8137.822 7858.358 7820.011 8051.076 ≈ 8010   ± 2.04% ↑ 2.01%
TPS    7819.831 7521.172 7519.023 7242.965 7426.000 ≈ 7506   ± 2.78% ↑ 0.79%
CPU %    41.983   38.775   40.606   39.375   35.537 ≈   39.3 ± 6.15% ↑ 5.65%
Mem MB 2947.189 2743.658 2896.688 2813.276 2863.108 ≈ 2853   ± 2.74% ↑ 0.85%
2021-08-06 21:10:34 +03:00
Roman Khimov
b55c75d59d network: hide Peers, make it return a slice
Slice is a bit more efficient, we don't need a map for Peers() users and it's
not really interesting to outside users, so better hide this method.
2021-08-06 21:10:34 +03:00
Roman Khimov
119b4200ac network: add fail-fast route for tx double processing
When transaction spreads through the network many nodes are likely to get it
in roughly the same time. They will rebroadcast it also in roughly the same
time. As we have a number of peers it's quite likely that we'd get an Inv with
the same transaction from multiple peers simultaneously. We will ask them for
this transaction (independently!) and again we're likely to get it in roughly
the same time. So we can easily end up with multiple threads processing the
same transaction. Only one will succeed, but we can actually easily avoid
doing it in the first place saving some CPU cycles for other things.

Notice that we can't do it _before_ receiving a transaction because nothing
guarantees that the peer will respond to our transaction request, so
communication overhead is unavoidable at the moment, but saving on processing
already gives quite interesting results.

Baseline, four nodes with 10 workers:

RPS    7176.784 7014.511 6139.663 7191.280 7080.852 ≈ 6921   ± 5.72%
TPS    6945.409 6562.756 5927.050 6681.187 6821.794 ≈ 6588   ± 5.38%
CPU %    44.400   43.842   40.418   49.211   49.370 ≈   45.4 ± 7.53%
Mem MB 2693.414 2640.602 2472.007 2731.482 2707.879 ≈ 2649   ± 3.53%

Patched:

RPS ≈  7791.675 7996.559 7834.504 7746.705 7891.614 ≈ 7852   ±  1.10% ↑ 13.45%
TPS ≈  7241.497 7711.765 7520.211 7425.890 7334.443 ≈ 7447   ±  2.17% ↑ 13.04%
CPU %    29.853   39.936   39.945   36.371   39.999 ≈   37.2 ± 10.57% ↓ 18.06%
Mem MB 2749.635 2791.609 2828.610 2910.431 2863.344 ≈ 2829   ±  1.97% ↑  6.80%
2021-08-06 21:10:25 +03:00
Roman Khimov
7fc153ed2a network: only ask mempool for intersections with received Inv
Most of the time on healthy network we see new transactions appearing that are
not present in the mempool. Once they get into mempool we don't ask for them
again when some other peer sends an Inv with them. Then these transactions are
usually added into block, removed from mempool and no one actually sends them
again to us. Some stale nodes can do that, but it's not very likely to
happen.

At the receiving end at the same time it's quite expensive to do full chain
HasTransaction() query, so if we can avoid doing that it's always good. Here
it technically allows resending old transaction that will be re-requested and
an attempt to add it to mempool will be made. But it'll inevitably fail
because the same HasTransaction() check is done there too. One can try to
maliciously flood the node with stale transactions but it doesn't differ from
flooding it with any other invalid transactions, so there is no new attack
vector added.

Baseline, 4 nodes with 10 workers:

RPS    6902.296 6465.662 6856.044 6785.515 6157.024 ≈ 6633   ± 4.26%
TPS    6468.431 6218.867 6610.565 6288.596 5790.556 ≈ 6275   ± 4.44%
CPU %    50.231   42.925   49.481   48.396   42.662 ≈   46.7 ± 7.01%
Mem MB 2856.841 2684.103 2756.195 2733.485 2422.787 ≈ 2691   ± 5.40%

Patched:

RPS    7176.784 7014.511 6139.663 7191.280 7080.852 ≈ 6921   ± 5.72% ↑ 4.34%
TPS    6945.409 6562.756 5927.050 6681.187 6821.794 ≈ 6588   ± 5.38% ↑ 4.99%
CPU %    44.400   43.842   40.418   49.211   49.370 ≈   45.4 ± 7.53% ↓ 2.78%
Mem MB 2693.414 2640.602 2472.007 2731.482 2707.879 ≈ 2649   ± 3.53% ↓ 1.56%
2021-08-06 20:53:02 +03:00
Roman Khimov
f9663a97a1 network: fix Ping messages
* NewPing() accepts block index first and nonce then.
 * Block height should be used, it'll be important for state exchanging nodes
2021-08-06 11:28:09 +03:00
Roman Khimov
1cea0dd894
Merge pull request #1997 from nspcc-dev/drop-syncreached-check
network: drop useless flag check
2021-06-04 23:39:34 +03:00
Roman Khimov
f6da88af0d network: drop useless flag check
It's the first thing done in tryStartServices(), so checking it here doesn't
make much sense.
2021-06-04 20:29:47 +03:00
Anna Shaleva
1dbf1d4310 rpc: allow to track notary requests via Notification subsystem 2021-06-01 16:29:04 +03:00
Roman Khimov
c4e084b0d8 *: fix whitespace errors
leading/trailing newlines
2021-05-12 22:51:41 +03:00
Roman Khimov
99108c620f network: fix errcheck warning 2021-05-12 20:14:35 +03:00
Roman Khimov
cfc067dd24 *: remove dead code
Found by deadcode via golangci-lint.
2021-05-12 18:13:14 +03:00
Evgeniy Stratonikov
275a5c9daa network: limit message number from the same sender 2021-05-12 10:52:11 +03:00
Anna Shaleva
09bb162de0 network: add ability to specify port for P2P version exchange 2021-04-30 11:27:55 +03:00
Roman Khimov
99b71bbbd1 network: move service starts to tryStartServices
All of them only make sense on a fully synchronized node, doing anything
during the initial sync is just a waste of time.
2021-04-02 13:12:06 +03:00