-
-
Notifications
You must be signed in to change notification settings - Fork 844
[client] Fix elapsed time calculation when machine is in sleep mode #4140
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
Conversation
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.
Pull Request Overview
This PR introduces a monotonic clock abstraction to avoid elapsed-time drift during sleep, migrating all last-activity tracking from time.Time
to monotime.Time
. It updates interfaces, the activity recorder, the inactivity monitor, and related tests, along with some logging adjustments.
- Added
monotime.Time
type,Now()
, andSince()
for stable monotonic timestamps. - Migrated
LastActivities()
APIs and their implementations to usemonotime.Time
. - Updated activity recorder, inactivity checks, and tests to employ
monotime.Since()
, and refined logging levels/messages.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
monotime/time.go | Added Time type, Now() , and Since() for monotonic time |
client/internal/lazyconn/wgiface.go | Changed LastActivities() to return monotime.Time |
client/internal/lazyconn/manager/manager.go | Updated peer activation logging and flow |
client/internal/lazyconn/inactivity/manager.go | Switched to monotime.Since() for inactivity detection |
client/internal/lazyconn/activity/listener.go | Adjusted logging levels and messages in listener |
client/internal/conn_mgr.go | Removed redundant activation log |
client/internal/iface_common.go | Updated interface to use monotime.Time |
client/iface/iface.go | Updated LastActivities() signature |
client/iface/device/interface.go | Updated LastActivities() signature |
client/iface/configurer/usp.go | Updated LastActivities() signature |
client/iface/configurer/kernel_unix.go | Updated LastActivities() , currently returns nil |
client/iface/bind/activity_test.go | Updated test to use monotime.Since() |
client/iface/bind/activity.go | Migrated recorder to store monotime.Time |
Comments suppressed due to low confidence (5)
monotime/time.go:12
- Add a doc comment for the
Time
type explaining that it represents a monotonic timestamp in nanoseconds since a fixed base.
type Time int64
monotime/time.go:28
- Add a doc comment for
Now()
to describe that it returns the current monotonic timestamp based on the package's base time.
func Now() Time {
client/internal/lazyconn/manager/manager.go:268
- Remove this unnecessary blank line to keep the code compact and consistent with surrounding formatting.
return true
client/internal/lazyconn/activity/listener.go:51
- [nitpick] This exit message is diagnostic—consider using
Debugf
instead ofInfof
to avoid cluttering info-level logs.
d.peerCfg.Log.Infof("exit from activity listener")
client/internal/lazyconn/activity/listener.go:62
- [nitpick] This event is internal; consider lowering the log level to
Debugf
to reduce noise in info-level logs.
d.peerCfg.Log.Infof("activity detected")
@@ -258,12 +258,13 @@ func (m *Manager) ActivatePeer(peerID string) (found bool) { | |||
return false | |||
} | |||
|
|||
cfg.Log.Infof("activate peer from inactive state by remote signal message") |
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.
[nitpick] Consider rephrasing this log message for clarity and consistency, e.g., Activating peer due to remote signal from inactive state
.
cfg.Log.Infof("activate peer from inactive state by remote signal message") | |
cfg.Log.Infof("Activating peer due to remote signal from inactive state") |
Copilot uses AI. Check for mistakes.
@@ -277,6 +279,6 @@ func (c *KernelConfigurer) GetStats() (map[string]WGStats, error) { | |||
return stats, nil | |||
} | |||
|
|||
func (c *KernelConfigurer) LastActivities() map[string]time.Time { | |||
func (c *KernelConfigurer) LastActivities() map[string]monotime.Time { | |||
return nil |
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.
Returning nil
for LastActivities()
may lead to unexpected nil-map usage; return an empty map (make(map[string]monotime.Time)
) instead.
return nil | |
return make(map[string]monotime.Time) |
Copilot uses AI. Check for mistakes.
|
Describe your changes
Issue ticket number and link
Stack
Checklist