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

chore(das/worker): rename log verb to be clear that not all headers are sampled #3196

Closed
wants to merge 2 commits into from

Conversation

ramin
Copy link
Contributor

@ramin ramin commented Feb 20, 2024

Addresses #3194 by suggesting we change the log message from "sampling" to "receiving". Other option could be "evaluating"? Somethign else?

@@ -86,7 +86,7 @@ func (w *worker) run(ctx context.Context, timeout time.Duration, resultCh chan<-

if w.state.jobType != recentJob {
log.Infow(
"finished sampling headers",
"finished receiving headers",
Copy link
Member

Choose a reason for hiding this comment

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

I think this would cause more confusion - "recency" in the context of DASer is related to whether or not a header coming in is a new networkHead (a header being gossiped). Headers that are !recent can still be sampled as long as they are > the sampling window cutoff.

@ramin ramin closed this Mar 4, 2024
@jcstein
Copy link
Member

jcstein commented Mar 4, 2024

so is it not sampling headers older than 30 days, and the logs are misleading?

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

Successfully merging this pull request may close these issues.

3 participants