syncapi: don't return early for no-op incremental syncs
* syncapi: don't return early for no-op incremental syncs
Comments explain why, but basically it's an inefficient use
of bandwidth and some sytests rely on /sync to block.
* Honour timeouts
* Actually return a response with timeout=0
MODIFY
syncapi/sync/requestpool.go
syncapi/sync/requestpool.go
@@ -251,8 +251,12 @@ func (rp *RequestPool) OnIncomingSyncRequest(req *http.Request, device *userapi.
waitingSyncRequests.Inc()
defer waitingSyncRequests.Dec()
+ // loop until we get some data
+ for {
+ startTime := time.Now()
currentPos := rp.Notifier.CurrentPosition()
+ // if the since token matches the current positions, wait via the notifier
if !rp.shouldReturnImmediately(syncReq, currentPos) {
timer := time.NewTimer(syncReq.Timeout) // case of timeout=0 is handled above
defer timer.Stop()
@@ -365,6 +369,27 @@ func (rp *RequestPool) OnIncomingSyncRequest(req *http.Request, device *userapi.
syncReq.Since.PresencePosition, currentPos.PresencePosition,
),
}
+ // it's possible for there to be no updates for this user even though since < current pos,
+ // e.g busy servers with a quiet user. In this scenario, we don't want to return a no-op
+ // response immediately, so let's try this again but pretend they bumped their since token.
+ // If the incremental sync was processed very quickly then we expect the next loop to block
+ // with a notifier, but if things are slow it's entirely possible that currentPos is no
+ // longer the current position so we will hit this code path again. We need to do this and
+ // not return a no-op response because:
+ // - It's an inefficient use of bandwidth.
+ // - Some sytests which test 'waking up' sync rely on some sync requests to block, which
+ // they weren't always doing, resulting in flakey tests.
+ if !syncReq.Response.HasUpdates() {
+ syncReq.Since = currentPos
+ // do not loop again if the ?timeout= is 0 as that means "return immediately"
+ if syncReq.Timeout > 0 {
+ syncReq.Timeout = syncReq.Timeout - time.Since(startTime)
+ if syncReq.Timeout < 0 {
+ syncReq.Timeout = 0
+ }
+ continue
+ }
+ }
}
return util.JSONResponse{
@@ -372,6 +397,7 @@ func (rp *RequestPool) OnIncomingSyncRequest(req *http.Request, device *userapi.
JSON: syncReq.Response,
}
}
+}
func (rp *RequestPool) OnIncomingKeyChangeRequest(req *http.Request, device *userapi.Device) util.JSONResponse {
from := req.URL.Query().Get("from")

MODIFY
syncapi/types/types.go
syncapi/types/types.go
@@ -350,6 +350,19 @@ type Response struct {
DeviceListsOTKCount map[string]int `json:"device_one_time_keys_count,omitempty"`
}
+func (r *Response) HasUpdates() bool {
+ // purposefully exclude DeviceListsOTKCount as we always include them
+ return (len(r.AccountData.Events) > 0 ||
+ len(r.Presence.Events) > 0 ||
+ len(r.Rooms.Invite) > 0 ||
+ len(r.Rooms.Join) > 0 ||
+ len(r.Rooms.Leave) > 0 ||
+ len(r.Rooms.Peek) > 0 ||
+ len(r.ToDevice.Events) > 0 ||
+ len(r.DeviceLists.Changed) > 0 ||
+ len(r.DeviceLists.Left) > 0)
+}
+
// NewResponse creates an empty response with initialised maps.
func NewResponse() *Response {
res := Response{}

syncapi: don't return early for no-op incremental syncs (#2473)

* syncapi: don't return early for no-op incremental syncs

Comments explain why, but basically it's an inefficient use
of bandwidth and some sytests rely on /sync to block.

* Honour timeouts

* Actually return a response with timeout=0