-
Notifications
You must be signed in to change notification settings - Fork 22
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
fix: Enhanced Data Collection and Response Time for SendWork and Twitter #471
Conversation
…sa-oracle into fix/tweet-errors
@jdutchak I added some changes to the timeouts and have commented in my review. When I run with my local node setup as a twitter scraper I only timeout even though my node should return a response in the Ideally in this case its should at minimum return the response from node node even if the other two nodes are unavailable to do the work. Or in the other case where my node cannot do the work the other three nodes in nodeData then return the work.
|
Hey @jdutchak quick note here when we catch errors from other nodes, suggest we add in the PeerID of the node from which the error is returning from and differentiate between
|
Looks like with a
|
Implement adaptive timeouts based on historical node performance
WorkerTimeout time.Time `json:"workerTimeout,omitempty"`
// Check WorkerTimeout
nodeData := node.NodeTracker.GetNodeData(p.PeerId.String())
if !nodeData.WorkerTimeout.IsZero() && time.Since(nodeData.WorkerTimeout) < 60*time.Minute {
logrus.Infof("[+] Skipping worker %s due to timeout", p.PeerId)
continue
}
// Set WorkerTimeout for the node
nodeData := node.NodeTracker.GetNodeData(data.ReceivedFrom.String())
if nodeData != nil {
nodeData.WorkerTimeout = time.Now()
node.NodeTracker.AddOrUpdateNodeData(nodeData, true)
}
func (net *NodeEventTracker) ClearExpiredWorkerTimeouts() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good. No issues for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great last catch. Happy you found it!
Description
✓ - denotes this task complete
This PR fixes # running tests for 429 and bad auth errors fixing timing for work responses to collector
Key Product Enhancements
Performance Metrics
Worker Selection:
Timeout Handling:
Response Collection:
Breaking Changes
Next Steps @jdutchak to document here
Notes for Reviewers
Signed commits