Skip to content
This repository has been archived by the owner on Aug 19, 2022. It is now read-only.

Only remove records during GC #135

Merged
merged 1 commit into from
Jul 17, 2021
Merged

Conversation

aarshkshah1992
Copy link
Contributor

@Stebalien

Continuation of #133.


// if we've expired all the signed addresses for a peer, remove their signed routing state record
if len(amap) == 0 {
delete(s.signedPeerRecords, p)
Copy link
Member

Choose a reason for hiding this comment

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

We should actually switch this to delete(s.addrs, p). Same below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Stebalien

Wont they be removed by GC ?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but there's no reason not to clean this up early if we know it's empty.

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

Meh. I'm fine with just removing this. Up to you.

But something is wrong with the tests.

@Stebalien Stebalien enabled auto-merge July 17, 2021 03:09
@Stebalien Stebalien merged commit eef7cec into master Jul 17, 2021
@aschmahmann aschmahmann mentioned this pull request Aug 23, 2021
62 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants