Skip to content
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

[neo2] Raise priority of Ping/Pong #1812

Merged
merged 5 commits into from
Aug 10, 2020

Conversation

Ashuaidehao
Copy link
Contributor

@Ashuaidehao Ashuaidehao commented Jul 31, 2020

Raise priority of Ping/Pong to ensure the reliability of syncing blocks process,Close #1806.

@shargon shargon requested a review from superboyiii July 31, 2020 07:30
vncoelho
vncoelho previously approved these changes Jul 31, 2020
Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good efforts. We need to port this to NEO 3 later.

@@ -217,8 +218,15 @@ public static Props Props(NeoSystem system)
return Akka.Actor.Props.Create(() => new TaskManager(system)).WithMailbox("task-manager-mailbox");
}

private readonly ConcurrentDictionary<ActorPath, DateTime> _expiredTimes = new ConcurrentDictionary<ActorPath, DateTime>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dictionary<ActorPath, DateTime> is enough?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better move it to L43.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dictionary<ActorPath, DateTime> is not thread-safe, may cause insert error in concurrent scenarios.

Copy link
Member

@shargon shargon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a mechanism for cleaning _expiredTimes

@Ashuaidehao
Copy link
Contributor Author

Maybe add a Timer ?

@shargon
Copy link
Member

shargon commented Aug 3, 2020

yes, a task it's good, something to prevent an outOfMemory.

shargon
shargon previously approved these changes Aug 4, 2020
@shargon shargon self-requested a review August 6, 2020 11:04
@shargon
Copy link
Member

shargon commented Aug 6, 2020

@Ashuaidehao please take a look to my solution for neo3 without a new collection. #1829 I think that we can do the same here

@shargon shargon closed this Aug 6, 2020
@shargon shargon reopened this Aug 6, 2020
@shargon
Copy link
Member

shargon commented Aug 6, 2020

Sorry for close, miss click

@shargon shargon changed the title Raise priority of Ping/Pong [neo2] Raise priority of Ping/Pong Aug 6, 2020
@shargon shargon added the port-to-3.x Feature or PR must be ported to Neo 3.x branch label Aug 6, 2020
@superboyiii
Copy link
Member

@shargon Test PASS, is able to merge. (from BSN side, the issue provider, they work well after this PR till now)

superboyiii
superboyiii previously approved these changes Aug 10, 2020
@Ashuaidehao Ashuaidehao dismissed stale reviews from superboyiii and shargon via 0e9a962 August 10, 2020 06:36
@erikzhang erikzhang merged commit 95596f9 into neo-project:master-2.x Aug 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
port-to-3.x Feature or PR must be ported to Neo 3.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants