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

implement decaying tags. #61

Merged
merged 8 commits into from
May 14, 2020
Merged

implement decaying tags. #61

merged 8 commits into from
May 14, 2020

Conversation

raulk
Copy link
Member

@raulk raulk commented Dec 26, 2019

Depends on libp2p/go-libp2p-core#104. See that PR for an explanation.

Tests use mocked time. We need to use mocked time more often.

Fixes #47.

Comment on lines +145 to +147
if err := cm.decayer.Close(); err != nil {
return err
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should still cancel the background goroutine, even if this fails.

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense.

// 1. Manages decay.
// 2. Applies score bumps.
// 3. Yields when closed.
func (d *decayer) process() {
Copy link
Contributor

Choose a reason for hiding this comment

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

A performance consideration here is that this loop might run way too often if the resolution is small.

Copy link
Member Author

Choose a reason for hiding this comment

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

True, maybe we should introduce a hard lower limit for the resolution?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that would work.

decay.go Show resolved Hide resolved
Copy link
Contributor

@vyzo vyzo left a comment

Choose a reason for hiding this comment

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

LGTM, modulo the cancellation of background goroutine in case of errors in Close.

decay.go Show resolved Hide resolved
raulk added 2 commits May 14, 2020 14:48
We now allow the user to request tags at an interval lower than
the decayer's resolution, and we override the effective interval
to be the resolution.

We also allow tags to have any interval, not necessarily a
multiple of the resolution any longer.
Copy link
Contributor

@yusefnapora yusefnapora left a comment

Choose a reason for hiding this comment

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

Awesome, thanks @raulk! This looks good to me :)

@raulk raulk merged commit 74d66a0 into master May 14, 2020
@raulk raulk deleted the feat/decayer branch May 14, 2020 16:34
@Stebalien Stebalien mentioned this pull request May 26, 2020
77 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.

Decaying weight
3 participants