sigb.us/dendrite
1
0
Fork 0
Code Issues Pull requests Actions Packages Projects Releases 1 Wiki Activity

Remove roominfo cache (#2615)

Browse source 
* Remove roominfo cache

It's the source of a number of race conditions which are seemingly causing bugs and CI failures.

* Make the linter less sad
This commit is contained in:
Neil Alexander 2022-08-03 17:14:21 +01:00 committed by GitHub
parent bbff41b44b
commit 2250768be1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 9 additions and 68 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

33
internal/caching/cache_roominfo.go
View file

@ -1,33 +0,0 @@
package caching
import (
"github.com/matrix-org/dendrite/roomserver/types"
)
// WARNING: This cache is mutable because it's entirely possible that
// the IsStub or StateSnaphotNID fields can change, even though the
// room version and room NID fields will not. This is only safe because
// the RoomInfoCache is used ONLY within the roomserver and because it
// will be kept up-to-date by the latest events updater. It MUST NOT be
// used from other components as we currently have no way to invalidate
// the cache in downstream components.
// RoomInfosCache contains the subset of functions needed for
// a room Info cache. It must only be used from the roomserver only
// It is not safe for use from other components.
type RoomInfoCache interface {
GetRoomInfo(roomID string) (roomInfo *types.RoomInfo, ok bool)
StoreRoomInfo(roomID string, roomInfo *types.RoomInfo)
}
// GetRoomInfo must only be called from the roomserver only. It is not
// safe for use from other components.
func (c Caches) GetRoomInfo(roomID string) (*types.RoomInfo, bool) {
return c.RoomInfos.Get(roomID)
}
// StoreRoomInfo must only be called from the roomserver only. It is not
// safe for use from other components.
func (c Caches) StoreRoomInfo(roomID string, roomInfo *types.RoomInfo) {
c.RoomInfos.Set(roomID, roomInfo)
}

1
internal/caching/cache_roomservernids.go
View file

@ -7,7 +7,6 @@ import (
type RoomServerCaches interface {
RoomServerNIDsCache
RoomVersionCache
RoomInfoCache
RoomServerEventsCache
EventStateKeyCache
}

1
internal/caching/caches.go
View file

@ -29,7 +29,6 @@ type Caches struct {
RoomServerRoomIDs Cache[types.RoomNID, string] // room NID -> room ID
RoomServerEvents Cache[int64, *gomatrixserverlib.Event] // event NID -> event
RoomServerStateKeys Cache[types.EventStateKeyNID, string] // event NID -> event state key
RoomInfos Cache[string, *types.RoomInfo] // room ID -> room info
FederationPDUs Cache[int64, *gomatrixserverlib.HeaderedEvent] // queue NID -> PDU
FederationEDUs Cache[int64, *gomatrixserverlib.EDU] // queue NID -> EDU
SpaceSummaryRooms Cache[string, gomatrixserverlib.MSC2946SpacesResponse] // room ID -> space response

7
internal/caching/impl_ristretto.go
View file

@ -35,7 +35,6 @@ const (
roomNIDsCache
roomIDsCache
roomEventsCache
roomInfosCache
federationPDUsCache
federationEDUsCache
spaceSummaryRoomsCache
@ -106,12 +105,6 @@ func NewRistrettoCache(maxCost config.DataUnit, maxAge time.Duration, enableProm
Prefix: eventStateKeyCache,
MaxAge: maxAge,
},
RoomInfos: &RistrettoCachePartition[string, *types.RoomInfo]{ // room ID -> room info
cache: cache,
Prefix: roomInfosCache,
Mutable: true,
MaxAge: maxAge,
},
FederationPDUs: &RistrettoCostedCachePartition[int64, *gomatrixserverlib.HeaderedEvent]{ // queue NID -> PDU
&RistrettoCachePartition[int64, *gomatrixserverlib.HeaderedEvent]{
cache: cache,

35
roomserver/storage/shared/storage.go
View file

@ -156,30 +156,15 @@ func (d *Database) RoomInfo(ctx context.Context, roomID string) (*types.RoomInfo
}
func (d *Database) roomInfo(ctx context.Context, txn *sql.Tx, roomID string) (*types.RoomInfo, error) {
roomInfo, ok := d.Cache.GetRoomInfo(roomID)
if ok && roomInfo != nil && !roomInfo.IsStub() {
// The data that's in the cache is not stubby, so return it.
return roomInfo, nil
}
// At this point we either don't have an entry in the cache, or
// it is stubby, so let's check the roomserver_rooms table again.
roomInfoFromDB, err := d.RoomsTable.SelectRoomInfo(ctx, txn, roomID)
roomInfo, err := d.RoomsTable.SelectRoomInfo(ctx, txn, roomID)
if err != nil {
return nil, err
}
// If we have a stubby cache entry already, update it and return
// the reference to the cache entry.
if roomInfo != nil {
roomInfo.CopyFrom(roomInfoFromDB)
return roomInfo, nil
d.Cache.StoreRoomServerRoomID(roomInfo.RoomNID, roomID)
d.Cache.StoreRoomVersion(roomID, roomInfo.RoomVersion)
}
// Otherwise, try to admit the data into the cache and return the
// new reference from the database.
if roomInfoFromDB != nil {
d.Cache.StoreRoomServerRoomID(roomInfoFromDB.RoomNID, roomID)
d.Cache.StoreRoomInfo(roomID, roomInfoFromDB)
}
return roomInfoFromDB, err
return roomInfo, err
}
func (d *Database) AddState(
@ -504,8 +489,8 @@ func (d *Database) events(
fetchNIDList := make([]types.RoomNID, 0, len(uniqueRoomNIDs))
for n := range uniqueRoomNIDs {
if roomID, ok := d.Cache.GetRoomServerRoomID(n); ok {
if roomInfo, ok := d.Cache.GetRoomInfo(roomID); ok {
roomVersions[n] = roomInfo.RoomVersion
if roomVersion, ok := d.Cache.GetRoomVersion(roomID); ok {
roomVersions[n] = roomVersion
continue
}
}
@ -762,9 +747,6 @@ func (d *Database) MissingAuthPrevEvents(
func (d *Database) assignRoomNID(
ctx context.Context, roomID string, roomVersion gomatrixserverlib.RoomVersion,
) (types.RoomNID, error) {
if roomInfo, ok := d.Cache.GetRoomInfo(roomID); ok {
return roomInfo.RoomNID, nil
}
// Check if we already have a numeric ID in the database.
roomNID, err := d.RoomsTable.SelectRoomNID(ctx, nil, roomID)
if err == sql.ErrNoRows {
@ -837,8 +819,9 @@ func extractRoomVersionFromCreateEvent(event *gomatrixserverlib.Event) (
// "servers should not apply or send redactions to clients until both the redaction event and original event have been seen, and are valid."
// https://matrix.org/docs/spec/rooms/v3#authorization-rules-for-events
// These cases are:
// - This is a redaction event, redact the event it references if we know about it.
// - This is a normal event which may have been previously redacted.
// - This is a redaction event, redact the event it references if we know about it.
// - This is a normal event which may have been previously redacted.
//
// In the first case, check if we have the referenced event then apply the redaction, else store it
// in the redactions table with validated=FALSE. In the second case, check if there is a redaction for it:
// if there is then apply the redactions and set validated=TRUE.