Skip to content

[neo2] Raise priority of Ping/Pong#1812

Merged
erikzhang merged 5 commits intoneo-project:master-2.xfrom
Ashuaidehao:Fix-1806
Aug 10, 2020
Merged

[neo2] Raise priority of Ping/Pong#1812
erikzhang merged 5 commits intoneo-project:master-2.xfrom
Ashuaidehao:Fix-1806

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.

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