From db39608bcbbcaf0ad2d3bad1ef75980e1a85d3a6 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Thu, 27 Feb 2020 02:13:31 -0500 Subject: [PATCH] change "last signoff" tracking to "last seen" Explicit quit and ping timeout behave the same way, but reattach after abandoning/losing the previous session (without the break being detected server-side) is more aggressive about replaying missed messages, at the cost of potential duplication. --- irc/accounts.go | 26 ++++++----- irc/channel.go | 4 +- irc/client.go | 98 +++++++++++++++++++++------------------- irc/client_lookup_set.go | 4 +- irc/commands.go | 5 +- irc/getters.go | 20 ++------ irc/idletimer.go | 12 ----- 7 files changed, 78 insertions(+), 91 deletions(-) diff --git a/irc/accounts.go b/irc/accounts.go index 4f7d4a5d..375668cd 100644 --- a/irc/accounts.go +++ b/irc/accounts.go @@ -35,7 +35,7 @@ const ( keyCertToAccount = "account.creds.certfp %s" keyAccountChannels = "account.channels %s" // channels registered to the account keyAccountJoinedChannels = "account.joinedto %s" // channels a persistent client has joined - keyAccountLastSignoff = "account.lastsignoff %s" + keyAccountLastSeen = "account.lastseen %s" keyVHostQueueAcctToId = "vhostQueue %s" vhostRequestIdx = "vhostQueue" @@ -104,7 +104,7 @@ func (am *AccountManager) createAlwaysOnClients(config *Config) { account, err := am.LoadAccount(accountName) if err == nil && account.Verified && persistenceEnabled(config.Accounts.Multiclient.AlwaysOn, account.Settings.AlwaysOn) { - am.server.AddAlwaysOnClient(account, am.loadChannels(accountName), am.loadLastSignoff(accountName)) + am.server.AddAlwaysOnClient(account, am.loadChannels(accountName), am.loadLastSeen(accountName)) } } } @@ -535,11 +535,11 @@ func (am *AccountManager) loadChannels(account string) (channels []string) { return } -func (am *AccountManager) saveLastSignoff(account string, lastSignoff time.Time) { - key := fmt.Sprintf(keyAccountLastSignoff, account) +func (am *AccountManager) saveLastSeen(account string, lastSeen time.Time) { + key := fmt.Sprintf(keyAccountLastSeen, account) var val string - if !lastSignoff.IsZero() { - val = strconv.FormatInt(lastSignoff.UnixNano(), 10) + if !lastSeen.IsZero() { + val = strconv.FormatInt(lastSeen.UnixNano(), 10) } am.server.store.Update(func(tx *buntdb.Tx) error { if val != "" { @@ -551,11 +551,15 @@ func (am *AccountManager) saveLastSignoff(account string, lastSignoff time.Time) }) } -func (am *AccountManager) loadLastSignoff(account string) (lastSignoff time.Time) { - key := fmt.Sprintf(keyAccountLastSignoff, account) +func (am *AccountManager) loadLastSeen(account string) (lastSeen time.Time) { + key := fmt.Sprintf(keyAccountLastSeen, account) var lsText string - am.server.store.View(func(tx *buntdb.Tx) error { + am.server.store.Update(func(tx *buntdb.Tx) error { lsText, _ = tx.Get(key) + // XXX clear this on startup, because it's not clear when it's + // going to be overwritten, and restarting the server twice in a row + // could result in a large amount of duplicated history replay + tx.Delete(key) return nil }) lsNum, err := strconv.ParseInt(lsText, 10, 64) @@ -1071,7 +1075,7 @@ func (am *AccountManager) Unregister(account string) error { vhostQueueKey := fmt.Sprintf(keyVHostQueueAcctToId, casefoldedAccount) channelsKey := fmt.Sprintf(keyAccountChannels, casefoldedAccount) joinedChannelsKey := fmt.Sprintf(keyAccountJoinedChannels, casefoldedAccount) - lastSignoffKey := fmt.Sprintf(keyAccountLastSignoff, casefoldedAccount) + lastSeenKey := fmt.Sprintf(keyAccountLastSeen, casefoldedAccount) var clients []*Client @@ -1108,7 +1112,7 @@ func (am *AccountManager) Unregister(account string) error { channelsStr, _ = tx.Get(channelsKey) tx.Delete(channelsKey) tx.Delete(joinedChannelsKey) - tx.Delete(lastSignoffKey) + tx.Delete(lastSeenKey) _, err := tx.Delete(vhostQueueKey) am.decrementVHostQueueCount(casefoldedAccount, err) diff --git a/irc/channel.go b/irc/channel.go index 7cedda92..2b79eabf 100644 --- a/irc/channel.go +++ b/irc/channel.go @@ -775,9 +775,9 @@ func (channel *Channel) autoReplayHistory(client *Client, rb *ResponseBuffer, sk var after, before time.Time if rb.session.zncPlaybackTimes != nil && (rb.session.zncPlaybackTimes.targets == nil || rb.session.zncPlaybackTimes.targets.Has(channel.NameCasefolded())) { after, before = rb.session.zncPlaybackTimes.after, rb.session.zncPlaybackTimes.before - } else if !rb.session.lastSignoff.IsZero() { + } else if !rb.session.autoreplayMissedSince.IsZero() { // we already checked for history caps in `playReattachMessages` - after = rb.session.lastSignoff + after = rb.session.autoreplayMissedSince } if !after.IsZero() || !before.IsZero() { diff --git a/irc/client.go b/irc/client.go index 5dedb650..0b0d2e57 100644 --- a/irc/client.go +++ b/irc/client.go @@ -47,7 +47,6 @@ type Client struct { accountName string // display name of the account: uncasefolded, '*' if not logged in accountRegDate time.Time accountSettings AccountSettings - atime time.Time away bool awayMessage string brbTimer BrbTimer @@ -60,7 +59,8 @@ type Client struct { invitedTo map[string]bool isSTSOnly bool languages []string - lastSignoff time.Time // for always-on clients, the time their last session quit + lastActive time.Time // last time they sent a command that wasn't PONG or similar + lastSeen time.Time // last time they sent any kind of command loginThrottle connection_limits.GenericThrottle nick string nickCasefolded string @@ -103,8 +103,8 @@ func (s *saslStatus) Clear() { type Session struct { client *Client - ctime time.Time - atime time.Time + ctime time.Time + lastActive time.Time socket *Socket realIP net.IP @@ -130,10 +130,10 @@ type Session struct { registrationMessages int - resumeID string - resumeDetails *ResumeDetails - zncPlaybackTimes *zncPlaybackTimes - lastSignoff time.Time + resumeID string + resumeDetails *ResumeDetails + zncPlaybackTimes *zncPlaybackTimes + autoreplayMissedSince time.Time batch MultilineBatch } @@ -247,11 +247,12 @@ func (server *Server) RunClient(conn clientConn, proxyLine string) { // give them 1k of grace over the limit: socket := NewSocket(conn.Conn, ircmsg.MaxlenTagsFromClient+512+1024, config.Server.MaxSendQBytes) client := &Client{ - atime: now, - channels: make(ChannelSet), - ctime: now, - isSTSOnly: conn.Config.STSOnly, - languages: server.Languages().Default(), + lastSeen: now, + lastActive: now, + channels: make(ChannelSet), + ctime: now, + isSTSOnly: conn.Config.STSOnly, + languages: server.Languages().Default(), loginThrottle: connection_limits.GenericThrottle{ Duration: config.Accounts.LoginThrottling.Duration, Limit: config.Accounts.LoginThrottling.MaxAttempts, @@ -270,7 +271,7 @@ func (server *Server) RunClient(conn clientConn, proxyLine string) { capVersion: caps.Cap301, capState: caps.NoneState, ctime: now, - atime: now, + lastActive: now, realIP: realIP, isTor: conn.Config.Tor, } @@ -306,24 +307,27 @@ func (server *Server) RunClient(conn clientConn, proxyLine string) { client.run(session, proxyLine) } -func (server *Server) AddAlwaysOnClient(account ClientAccount, chnames []string, lastSignoff time.Time) { +func (server *Server) AddAlwaysOnClient(account ClientAccount, chnames []string, lastActive time.Time) { now := time.Now().UTC() config := server.Config() + if lastActive.IsZero() { + lastActive = now + } client := &Client{ - atime: now, - channels: make(ChannelSet), - ctime: now, - languages: server.Languages().Default(), - server: server, + lastSeen: now, + lastActive: lastActive, + channels: make(ChannelSet), + ctime: now, + languages: server.Languages().Default(), + server: server, // TODO figure out how to set these on reattach? username: "~user", rawHostname: server.name, realIP: utils.IPv4LoopbackAddress, - alwaysOn: true, - lastSignoff: lastSignoff, + alwaysOn: true, } client.SetMode(modes.TLS, true) @@ -662,25 +666,30 @@ func (client *Client) playReattachMessages(session *Session) { channel.autoReplayHistory(client, rb, "") rb.Send(true) } - if !session.lastSignoff.IsZero() && !hasHistoryCaps { + if !session.autoreplayMissedSince.IsZero() && !hasHistoryCaps { rb := NewResponseBuffer(session) - zncPlayPrivmsgs(client, rb, session.lastSignoff, time.Time{}) + zncPlayPrivmsgs(client, rb, session.autoreplayMissedSince, time.Time{}) rb.Send(true) } - session.lastSignoff = time.Time{} + session.autoreplayMissedSince = time.Time{} } // // idle, quit, timers and timeouts // -// Active updates when the client was last 'active' (i.e. the user should be sitting in front of their client). -func (client *Client) Active(session *Session) { +// Touch indicates that we received a line from the client (so the connection is healthy +// at this time, modulo network latency and fakelag). `active` means not a PING or suchlike +// (i.e. the user should be sitting in front of their client). +func (client *Client) Touch(active bool, session *Session) { now := time.Now().UTC() client.stateMutex.Lock() defer client.stateMutex.Unlock() - session.atime = now - client.atime = now + client.lastSeen = now + if active { + client.lastActive = now + session.lastActive = now + } } // Ping sends the client a PING message. @@ -896,7 +905,7 @@ func (client *Client) replayPrivmsgHistory(rb *ResponseBuffer, items []history.I func (client *Client) IdleTime() time.Duration { client.stateMutex.RLock() defer client.stateMutex.RUnlock() - return time.Since(client.atime) + return time.Since(client.lastActive) } // SignonTime returns this client's signon time as a unix timestamp. @@ -1151,12 +1160,6 @@ func (client *Client) Quit(message string, session *Session) { // has no more sessions. func (client *Client) destroy(session *Session) { var sessionsToDestroy []*Session - var lastSignoff time.Time - if session != nil { - lastSignoff = session.idletimer.LastTouch() - } else { - lastSignoff = time.Now().UTC() - } client.stateMutex.Lock() details := client.detailsNoMutex() @@ -1166,6 +1169,7 @@ func (client *Client) destroy(session *Session) { sessionRemoved := false registered := client.registered alwaysOn := client.alwaysOn + saveLastSeen := alwaysOn && client.accountSettings.AutoreplayMissed var remainingSessions int if session == nil { sessionsToDestroy = client.sessions @@ -1187,16 +1191,17 @@ func (client *Client) destroy(session *Session) { // if it's our job to destroy it, don't let anyone else try client.destroyed = true } - if alwaysOn && remainingSessions == 0 { - client.lastSignoff = lastSignoff - client.dirtyBits |= IncludeLastSignoff - } else { - lastSignoff = time.Time{} + if saveLastSeen { + client.dirtyBits |= IncludeLastSeen } exitedSnomaskSent := client.exitedSnomaskSent client.stateMutex.Unlock() - if !lastSignoff.IsZero() { + // XXX there is no particular reason to persist this state here rather than + // any other place: it would be correct to persist it after every `Touch`. However, + // I'm not comfortable introducing that many database writes, and I don't want to + // design a throttle. + if saveLastSeen { client.wakeWriter() } @@ -1571,10 +1576,9 @@ func (client *Client) historyStatus(config *Config) (status HistoryStatus, targe // these are bit flags indicating what part of the client status is "dirty" // and needs to be read from memory and written to the db -// TODO add a dirty flag for lastSignoff const ( IncludeChannels uint = 1 << iota - IncludeLastSignoff + IncludeLastSeen ) func (client *Client) markDirty(dirtyBits uint) { @@ -1629,10 +1633,10 @@ func (client *Client) performWrite() { } client.server.accounts.saveChannels(account, channelNames) } - if (dirtyBits & IncludeLastSignoff) != 0 { + if (dirtyBits & IncludeLastSeen) != 0 { client.stateMutex.RLock() - lastSignoff := client.lastSignoff + lastSeen := client.lastSeen client.stateMutex.RUnlock() - client.server.accounts.saveLastSignoff(account, lastSignoff) + client.server.accounts.saveLastSeen(account, lastSeen) } } diff --git a/irc/client_lookup_set.go b/irc/client_lookup_set.go index ae23f930..799d4b05 100644 --- a/irc/client_lookup_set.go +++ b/irc/client_lookup_set.go @@ -174,7 +174,7 @@ func (clients *ClientManager) SetNick(client *Client, session *Session, newNick if registered || !bouncerAllowed || account == "" || account != currentClient.Account() || client.HasMode(modes.TLS) != currentClient.HasMode(modes.TLS) { return "", errNicknameInUse } - reattachSuccessful, numSessions, lastSignoff := currentClient.AddSession(session) + reattachSuccessful, numSessions, lastSeen := currentClient.AddSession(session) if !reattachSuccessful { return "", errNicknameInUse } @@ -183,7 +183,7 @@ func (clients *ClientManager) SetNick(client *Client, session *Session, newNick operator := client.HasMode(modes.Operator) || client.HasMode(modes.LocalOperator) client.server.stats.AddRegistered(invisible, operator) } - session.lastSignoff = lastSignoff + session.autoreplayMissedSince = lastSeen // XXX SetNames only changes names if they are unset, so the realname change only // takes effect on first attach to an always-on client (good), but the user/ident // change is always a no-op (bad). we could make user/ident act the same way as diff --git a/irc/commands.go b/irc/commands.go index a899b571..cdc7804f 100644 --- a/irc/commands.go +++ b/irc/commands.go @@ -65,8 +65,9 @@ func (cmd *Command) Run(server *Server, client *Client, session *Session, msg ir session.idletimer.Touch() } - if !exiting && client.registered && !cmd.leaveClientIdle { - client.Active(session) + // TODO: eliminate idletimer entirely in favor of this measurement + if client.registered { + client.Touch(!cmd.leaveClientIdle, session) } return exiting diff --git a/irc/getters.go b/irc/getters.go index 1997f6ee..cd1a75d1 100644 --- a/irc/getters.go +++ b/irc/getters.go @@ -79,7 +79,7 @@ func (client *Client) AllSessionData(currentSession *Session) (data []SessionDat currentIndex = i } data[i] = SessionData{ - atime: session.atime, + atime: session.lastActive, ctime: session.ctime, hostname: session.rawHostname, certfp: session.certfp, @@ -93,13 +93,7 @@ func (client *Client) AllSessionData(currentSession *Session) (data []SessionDat return } -func (client *Client) AddSession(session *Session) (success bool, numSessions int, lastSignoff time.Time) { - defer func() { - if !lastSignoff.IsZero() { - client.wakeWriter() - } - }() - +func (client *Client) AddSession(session *Session) (success bool, numSessions int, lastSeen time.Time) { client.stateMutex.Lock() defer client.stateMutex.Unlock() @@ -112,15 +106,11 @@ func (client *Client) AddSession(session *Session) (success bool, numSessions in newSessions := make([]*Session, len(client.sessions)+1) copy(newSessions, client.sessions) newSessions[len(newSessions)-1] = session - if len(client.sessions) == 0 && client.accountSettings.AutoreplayMissed { - // n.b. this is only possible if client is persistent and remained - // on the server with no sessions: - lastSignoff = client.lastSignoff - client.lastSignoff = time.Time{} - client.dirtyBits |= IncludeLastSignoff + if client.accountSettings.AutoreplayMissed { + lastSeen = client.lastSeen } client.sessions = newSessions - return true, len(client.sessions), lastSignoff + return true, len(client.sessions), lastSeen } func (client *Client) removeSession(session *Session) (success bool, length int) { diff --git a/irc/idletimer.go b/irc/idletimer.go index fe7e7321..da2f6fb2 100644 --- a/irc/idletimer.go +++ b/irc/idletimer.go @@ -52,7 +52,6 @@ type IdleTimer struct { quitTimeout time.Duration state TimerState timer *time.Timer - lastTouch time.Time } // Initialize sets up an IdleTimer and starts counting idle time; @@ -62,11 +61,9 @@ func (it *IdleTimer) Initialize(session *Session) { it.registerTimeout = RegisterTimeout it.idleTimeout, it.quitTimeout = it.recomputeDurations() registered := session.client.Registered() - now := time.Now().UTC() it.Lock() defer it.Unlock() - it.lastTouch = now if registered { it.state = TimerActive } else { @@ -95,12 +92,10 @@ func (it *IdleTimer) recomputeDurations() (idleTimeout, quitTimeout time.Duratio func (it *IdleTimer) Touch() { idleTimeout, quitTimeout := it.recomputeDurations() - now := time.Now().UTC() it.Lock() defer it.Unlock() it.idleTimeout, it.quitTimeout = idleTimeout, quitTimeout - it.lastTouch = now // a touch transitions TimerUnregistered or TimerIdle into TimerActive if it.state != TimerDead { it.state = TimerActive @@ -108,13 +103,6 @@ func (it *IdleTimer) Touch() { } } -func (it *IdleTimer) LastTouch() (result time.Time) { - it.Lock() - result = it.lastTouch - it.Unlock() - return -} - func (it *IdleTimer) processTimeout() { idleTimeout, quitTimeout := it.recomputeDurations()