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

refactoring #288

Merged
merged 53 commits into from Sep 14, 2018

Conversation

@erikzhang
Member

erikzhang commented Jun 25, 2018

Refactor the code using the actor model. (Akka.NET)

Close #282
Close neo-project/neo-cli#116

@erikzhang erikzhang added this to the NEO 3.0 milestone Jun 25, 2018

@erikzhang erikzhang requested a review from shargon Jun 25, 2018

// if (engine.EvaluationStack.Count != 1 || !engine.EvaluationStack.Pop().GetBoolean()) return false;
// }
//}
return true;

This comment has been minimized.

@AshRolls

AshRolls Jun 25, 2018

Member

Remove commented code if no longer used.

private readonly Dictionary<IPAddress, int> ConnectedAddresses = new Dictionary<IPAddress, int>();
protected readonly Dictionary<IActorRef, IPEndPoint> ConnectedPeers = new Dictionary<IActorRef, IPEndPoint>();
protected readonly HashSet<IPEndPoint> UnconnectedPeers = new HashSet<IPEndPoint>();
//TODO: badPeers

This comment has been minimized.

@AshRolls

AshRolls Jun 25, 2018

Member

TODO ?

catch { }
}
tcp.Tell(new Tcp.Bind(Self, new IPEndPoint(IPAddress.Any, ListenerPort), options: new[] { new Inet.SO.ReuseAddress(true) }));
//TODO: Websocket

This comment has been minimized.

@AshRolls
// peerJson["address"] = peer.Address.ToString();
// peerJson["port"] = peer.Port;
// unconnectedPeers.Add(peerJson);
//}

This comment has been minimized.

@AshRolls

AshRolls Jun 25, 2018

Member

Commented code / TODO

// peerJson["address"] = peer.Address.ToString();
// peerJson["port"] = peer.Port;
// badPeers.Add(peerJson);
//}

This comment has been minimized.

@AshRolls

AshRolls Jun 25, 2018

Member

Commented Code / TODO

@vncoelho

This comment has been minimized.

Member

vncoelho commented Jun 25, 2018

OMG, @erikzhang.

{
Log($"send perpare response");
Log($"send prepare response");

This comment has been minimized.

@vncoelho

vncoelho Jun 25, 2018

Member

I was waiting for this...aehuaheuaea

Show resolved Hide resolved neo/Consensus/ConsensusService.cs
}
TimeSpan span = DateTime.Now - block_received_time;

This comment has been minimized.

@vncoelho

vncoelho Jun 25, 2018

Member

Erik, let's consider a startup time here.

TimeSpan span = DateTime.Now + averageStartupTimeTimeOfKPreviousBlocks - block_received_time;

But, perhaps, not now, right? It would make harder to detect any different in this refactoring.

This comment has been minimized.

@shargon

shargon Jun 26, 2018

Member

DateTime.UtcNow please

This comment has been minimized.

@vncoelho

vncoelho Jun 27, 2018

Member

What is the difference, Sharrrgon brothers?

This comment has been minimized.

@vncoelho

vncoelho Jun 27, 2018

Member

The important thing here would be to have something unified in terms of clock and time syncing. The template has almost not so much effects.

Show resolved Hide resolved neo/Consensus/ConsensusService.cs Outdated
Show resolved Hide resolved neo/Implementations/Blockchains/LevelDB/LevelDBBlockchain.cs
Show resolved Hide resolved neo/Cryptography/Base58.cs
Show resolved Hide resolved neo/IO/Json/JString.cs
@vncoelho

This comment has been minimized.

Member

vncoelho commented on neo/IO/Actors/PriorityMessageQueue.cs in eef4231 Jun 28, 2018

Hi, Z. Erik,
There is a design that metaheuristics programmer like.
I do not know the feasibility. But, please, take a look.

Instead of two Queues (high and low), consider only 1 Queue of a Pair <Envelope, weight>!
Let's say, weight is in [0,1], where 0 is low and 1 is high priority.

A Quicksort algorithm could sort this Queue quite "Quick". haeujhauehauea
Igor, help me with the calculations, because, perhaps, the Queue can growth until something around 20-30k or even more, is that right, Erik?

In particular, Erik, this design opens important doors for optimization strategies.

  1. Different strategies could be used to prioritize transactions easily, only playing with weights.
  2. With this sorted vector we can plan with the transaction that will be included in a Greedy Randomized manner, for example (with an additional alpha parameter also from [0,1] for example, in which 0 is random and 1 is greedy).

PS: Just now I realized that this queue might is not related to transaction and only the messages =/ Anyway...

This comment has been minimized.

Member

erikzhang replied Jun 28, 2018

If you sort the messages, it means that you need to lock the queue every time you enqueue message. This is a synchronous operation, which can greatly reduce the performance.

@vncoelho

This comment has been minimized.

Member

vncoelho commented on neo/Consensus/ConsensusService.cs in eef4231 Jun 28, 2018

HIgh priority will be messages related to consensus messages and Block Persisted. Nice catch, Erik.
In this sense, transactions and other messages would be processed later, right?

I only took at after my comment about the Queue of a pair.

@erikzhang

This comment has been minimized.

Member

erikzhang commented Jul 2, 2018

Now we can test this PR with neo-gui and neo-cli.

@vncoelho

This comment has been minimized.

Member

vncoelho commented Jul 2, 2018

Testing are starting: NeoResearch/neo-tests@95e5e49
Thanks @AshRolls 🗡

@vncoelho

This comment has been minimized.

Member

vncoelho commented Jul 2, 2018

Great implementation, @erikzhang, congratulations for the concise modification. Apparently, basic functionalities are working pretty smooth.
NeoCompiler is very interlaced and everything seems to be working in the desired manner.
Notoriously, I fell that things are running softer. However, it is a biased analyses. aheuahueaea

image

I noticed that 29 free transactions were broadcasted in a single block, perhaps, I should adjust the freemaxtransaction per block here.

We gonna check it with time and verify the boundaries defined in Neo 3.0

Checking with additional features, I am getting the following errors:

[ERROR][07/02/2018 16:48:03][Thread 0006][akka://NeoSystem/user/$b/connection_c13547dc-7bc6-48db-bdd3-0a1bd009a2f6/$a] Operation is not valid due to the current state of the object.
Cause: System.Net.ProtocolViolationException: Operation is not valid due to the current state of the object.
   at Neo.Network.P2P.ProtocolHandler.OnReceive(Object message)
   at Akka.Actor.UntypedActor.Receive(Object message)
   at Akka.Actor.ActorBase.AroundReceive(Receive receive, Object message)
   at Akka.Actor.ActorCell.ReceiveMessage(Object message)
   at Akka.Actor.ActorCell.Invoke(Envelope envelope)

[ERROR][07/02/2018 16:48:03][Thread 0010][akka://NeoSystem/system/IO-TCP/$g] Transport endpoint is not connected
Cause: System.Net.Sockets.SocketException (0x80004005): Transport endpoint is not connected
   at System.Net.Sockets.Socket.UpdateStatusAfterSocketErrorAndThrowException(SocketError error, String callerName)
   at System.Net.Sockets.Socket.get_RemoteEndPoint()
   at Akka.IO.TcpConnection.CompleteConnect(IActorRef commander, IEnumerable`1 options)
   at Akka.IO.TcpOutgoingConnection.<>c__DisplayClass12_0.<Connecting>b__0(Object message)
   at Akka.Actor.ActorBase.AroundReceive(Receive receive, Object message)
   at Akka.Actor.ActorCell.ReceiveMessage(Object message)
   at Akka.Actor.ActorCell.Invoke(Envelope envelope)
[ERROR][07/02/2018 16:48:03][Thread 0005][akka://NeoSystem/system/IO-TCP/$f] Restarting not supported for connection actors.
Cause: [akka://NeoSystem/system/IO-TCP/$f#1048723336]: Akka.Actor.PostRestartException: Exception post restart (System.Net.Sockets.SocketException) ---> Akka.Pattern.IllegalStateException: Restarting not supported for connection actors.
   at Akka.IO.TcpConnection.PostRestart(Exception reason)
   at Akka.Actor.ActorCell.UseThreadContext(Action action)
   at Akka.Actor.ActorCell.FinishRecreate(Exception cause, ActorBase failedActor)
   --- End of inner exception stack trace ---
@vncoelho

This comment has been minimized.

Member

vncoelho commented on neo/Network/P2P/Peer.cs in 09878c1 Jul 2, 2018

Is the WS being used now? If yes, will it be an option or we just should set the doors correctly?

The seed should keep point to the TCP/IP?

erikzhang added some commits Jul 3, 2018

update dependencies:
- Microsoft.AspNetCore.ResponseCompression v2.1.1
- Microsoft.AspNetCore.Server.Kestrel v2.1.1
- Microsoft.AspNetCore.Server.Kestrel.Https v2.1.1
- Microsoft.EntityFrameworkCore.Sqlite v2.1.1
- Microsoft.Extensions.Configuration.Json v2.1.1
@vncoelho

This comment has been minimized.

Member

vncoelho commented Jul 3, 2018

Great, Erik.

RPC calls are now responding. However, still stucked on start consensus.

image

image

Is it compulsory to include SimplePolicy.cs plugin?

@erikzhang

This comment has been minimized.

Member

erikzhang commented Jul 3, 2018

This is a caught exception, no problem.

Show resolved Hide resolved neo/Ledger/Blockchain.cs Outdated
Show resolved Hide resolved neo/Network/P2P/Peer.cs
Show resolved Hide resolved neo/Network/P2P/Peer.cs
@vncoelho

This comment has been minimized.

Member

vncoelho commented Sep 11, 2018

Great work, @erikzhang.
Incredible improvements surely were held here!
Pretty stable. Good move on the backwards compatibility, @jsolman.

The last release can be found at https://github.com/NeoResearch/neo-tests/blob/master/releases/neo-release-2.9.0-preview-all-plugins.zip

@jsolman

This comment has been minimized.

Contributor

jsolman commented Sep 12, 2018

Yes, overall, it is cleaner code and there are some nice improvements. I have been having good results testing so far in PrivateNet. I'll be launching some nodes on TestNet and observing metrics and will report back some findings here within the next day or two.

@jsolman

This comment has been minimized.

Contributor

jsolman commented Sep 12, 2018

I'm experiencing an issue now where sometimes in my private net when I stop and start the server it starts syncing the chain from genesis block instead of continuing from the most recent persisted block in leveldb. Anyone else seen this?

Show resolved Hide resolved neo/Persistence/LevelDB/LevelDBStore.cs Outdated
@vncoelho

This comment has been minimized.

Member

vncoelho commented Sep 12, 2018

@jsolman, our node did not presented sync problems, if the "index" param is added in the config ( "Index": "Index_{0}").

@jsolman

This comment has been minimized.

Contributor

jsolman commented Sep 12, 2018

@vncoelho Erik pushed a fix to the sync issue I was mentioning two hours before your comment.

@jsolman

This comment has been minimized.

Contributor

jsolman commented Sep 13, 2018

Some transactions submitted successively via sendrawtransaction to a node running this code seem to be getting lost; the node accepts all the transactions, but after waiting multiple blocks not all of them get confirmed on the consensus nodes in my private net. I'm not sure what the problem is yet, but lets get some others testing this scenario also. In my case the consensus nodes are running 2.8.0 or less

I verified the messages are being relayed from the node running this code; so this code may be fine; still not sure what is happening to the messages just yet.

@jsolman

This comment has been minimized.

Contributor

jsolman commented Sep 13, 2018

After further investigation, if I send the same 6 transactions using sendrawtransaction to the consensus node they all get confirmed without any issue. If I send them to the node running 2.9.0 and it relays them it relays them all, but only 3 or 4 out of 6 actually make it into a block. The last ones stay in the mem_pool on the 2.9.0 node getting re-verified after every block indefinitely and the consensus nodes never confirm them.

I am digging deeper to figure out why they get confirmed if sent directly to the consensus node but when using sendrawtransaction to the 2.9.0 node some never will get confirmed.

Show resolved Hide resolved neo/Ledger/Blockchain.cs Outdated
Show resolved Hide resolved neo/Ledger/Blockchain.cs Outdated

erikzhang added some commits Sep 13, 2018

erikzhang and others added some commits Sep 13, 2018

@erikzhang erikzhang merged commit c64748e into master Sep 14, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@erikzhang erikzhang deleted the Neo.Actor branch Sep 14, 2018

@jsolman

This comment has been minimized.

Contributor

jsolman commented Sep 14, 2018

Sweet to see this merged!

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