Merge pull request #828 from slingamn/sessionrace.1

change "last signoff" tracking to "last seen"

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)

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() {

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)
 	}
 }

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

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

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) {

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()