Skip to content
This repository was archived by the owner on Nov 25, 2024. It is now read-only.

Commit 61341ac

Browse files
S7evinKS7evinK
authored
Add tests for the UpDropEventReferenceSHAPrevEvents migration (#3087)
... as they could fail if there are duplicate events in `roomserver_previous_events`. This fixes the migration by trying to combine the `event_nids` if possible (same room) as mentioned by @kegsay in #3083 (comment)
1 parent 3dcca40 commit 61341ac

File tree

4 files changed

+271
-12
lines changed

4 files changed

+271
-12
lines changed

‎roomserver/storage/postgres/deltas/20230516154000_drop_reference_sha.go

Lines changed: 76 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,14 @@ import (
1818
"context"
1919
"database/sql"
2020
"fmt"
21+
22+
"github.com/lib/pq"
23+
"github.com/matrix-org/dendrite/internal"
24+
"github.com/matrix-org/util"
2125
)
2226

2327
func UpDropEventReferenceSHAEvents(ctx context.Context, tx *sql.Tx) error {
24-
var count int
25-
err := tx.QueryRowContext(ctx, `SELECT count(*) FROM roomserver_events GROUP BY event_id HAVING count(event_id) > 1`).
26-
Scan(&count)
27-
if err != nil && err != sql.ErrNoRows {
28-
return fmt.Errorf("failed to query duplicate event ids")
29-
}
30-
if count > 0 {
31-
return fmt.Errorf("unable to drop column, as there are duplicate event ids")
32-
}
33-
_, err = tx.ExecContext(ctx, `ALTER TABLE roomserver_events DROP COLUMN IF EXISTS reference_sha256;`)
28+
_, err := tx.ExecContext(ctx, `ALTER TABLE roomserver_events DROP COLUMN IF EXISTS reference_sha256;`)
3429
if err != nil {
3530
return fmt.Errorf("failed to execute upgrade: %w", err)
3631
}
@@ -46,9 +41,80 @@ func UpDropEventReferenceSHAPrevEvents(ctx context.Context, tx *sql.Tx) error {
4641
if err != nil {
4742
return fmt.Errorf("failed to execute upgrade: %w", err)
4843
}
44+
45+
// figure out if there are duplicates
46+
dupeRows, err := tx.QueryContext(ctx, `SELECT previous_event_id FROM roomserver_previous_events GROUP BY previous_event_id HAVING count(previous_event_id) > 1`)
47+
if err != nil {
48+
return fmt.Errorf("failed to query duplicate event ids")
49+
}
50+
defer internal.CloseAndLogIfError(ctx, dupeRows, "failed to close rows")
51+
52+
var prevEvents []string
53+
var prevEventID string
54+
for dupeRows.Next() {
55+
if err = dupeRows.Scan(&prevEventID); err != nil {
56+
return err
57+
}
58+
prevEvents = append(prevEvents, prevEventID)
59+
}
60+
if dupeRows.Err() != nil {
61+
return dupeRows.Err()
62+
}
63+
64+
// if we found duplicates, check if we can combine them, e.g. they are in the same room
65+
for _, dupeID := range prevEvents {
66+
var dupeNIDsRows *sql.Rows
67+
dupeNIDsRows, err = tx.QueryContext(ctx, `SELECT event_nids FROM roomserver_previous_events WHERE previous_event_id = $1`, dupeID)
68+
if err != nil {
69+
return fmt.Errorf("failed to query duplicate event ids")
70+
}
71+
defer internal.CloseAndLogIfError(ctx, dupeNIDsRows, "failed to close rows")
72+
var dupeNIDs []int64
73+
for dupeNIDsRows.Next() {
74+
var nids pq.Int64Array
75+
if err = dupeNIDsRows.Scan(&nids); err != nil {
76+
return err
77+
}
78+
dupeNIDs = append(dupeNIDs, nids...)
79+
}
80+
81+
if dupeNIDsRows.Err() != nil {
82+
return dupeNIDsRows.Err()
83+
}
84+
// dedupe NIDs
85+
dupeNIDs = dupeNIDs[:util.SortAndUnique(nids(dupeNIDs))]
86+
// now that we have all NIDs, check which room they belong to
87+
var roomCount int
88+
err = tx.QueryRowContext(ctx, `SELECT count(distinct room_nid) FROM roomserver_events WHERE event_nid = ANY($1)`, pq.Array(dupeNIDs)).Scan(&roomCount)
89+
if err != nil {
90+
return err
91+
}
92+
// if the events are from different rooms, that's bad and we can't continue
93+
if roomCount > 1 {
94+
return fmt.Errorf("detected events (%v) referenced for different rooms (%v)", dupeNIDs, roomCount)
95+
}
96+
// otherwise delete the dupes
97+
_, err = tx.ExecContext(ctx, "DELETE FROM roomserver_previous_events WHERE previous_event_id = $1", dupeID)
98+
if err != nil {
99+
return fmt.Errorf("unable to delete duplicates: %w", err)
100+
}
101+
102+
// insert combined values
103+
_, err = tx.ExecContext(ctx, "INSERT INTO roomserver_previous_events (previous_event_id, event_nids) VALUES ($1, $2)", dupeID, pq.Array(dupeNIDs))
104+
if err != nil {
105+
return fmt.Errorf("unable to insert new event NIDs: %w", err)
106+
}
107+
}
108+
49109
_, err = tx.ExecContext(ctx, `ALTER TABLE roomserver_previous_events ADD CONSTRAINT roomserver_previous_event_id_unique UNIQUE (previous_event_id);`)
50110
if err != nil {
51111
return fmt.Errorf("failed to execute upgrade: %w", err)
52112
}
53113
return nil
54114
}
115+
116+
type nids []int64
117+
118+
func (s nids) Len() int { return len(s) }
119+
func (s nids) Less(i, j int) bool { return s[i] < s[j] }
120+
func (s nids) Swap(i, j int) { s[i], s[j] = s[j], s[i] }

‎roomserver/storage/postgres/deltas/20230516154000_drop_reference_sha_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package deltas
2+
3+
import (
4+
"testing"
5+
6+
"github.com/lib/pq"
7+
"github.com/matrix-org/dendrite/internal/sqlutil"
8+
"github.com/matrix-org/dendrite/test"
9+
"github.com/matrix-org/dendrite/test/testrig"
10+
"github.com/stretchr/testify/assert"
11+
)
12+
13+
func TestUpDropEventReferenceSHAPrevEvents(t *testing.T) {
14+
15+
cfg, ctx, close := testrig.CreateConfig(t, test.DBTypePostgres)
16+
defer close()
17+
18+
db, err := sqlutil.Open(&cfg.Global.DatabaseOptions, sqlutil.NewDummyWriter())
19+
assert.Nil(t, err)
20+
assert.NotNil(t, db)
21+
defer db.Close()
22+
23+
// create the table in the old layout
24+
_, err = db.ExecContext(ctx.Context(), `
25+
CREATE TABLE IF NOT EXISTS roomserver_previous_events (
26+
previous_event_id TEXT NOT NULL,
27+
event_nids BIGINT[] NOT NULL,
28+
previous_reference_sha256 BYTEA NOT NULL,
29+
CONSTRAINT roomserver_previous_event_id_unique UNIQUE (previous_event_id, previous_reference_sha256)
30+
);`)
31+
assert.Nil(t, err)
32+
33+
// create the events table as well, slimmed down with one eventNID
34+
_, err = db.ExecContext(ctx.Context(), `
35+
CREATE SEQUENCE IF NOT EXISTS roomserver_event_nid_seq;
36+
CREATE TABLE IF NOT EXISTS roomserver_events (
37+
event_nid BIGINT PRIMARY KEY DEFAULT nextval('roomserver_event_nid_seq'),
38+
room_nid BIGINT NOT NULL
39+
);
40+
41+
INSERT INTO roomserver_events (event_nid, room_nid) VALUES (1, 1)
42+
`)
43+
assert.Nil(t, err)
44+
45+
// insert duplicate prev events with different event_nids
46+
stmt, err := db.PrepareContext(ctx.Context(), `INSERT INTO roomserver_previous_events (previous_event_id, event_nids, previous_reference_sha256) VALUES ($1, $2, $3)`)
47+
assert.Nil(t, err)
48+
assert.NotNil(t, stmt)
49+
_, err = stmt.ExecContext(ctx.Context(), "1", pq.Array([]int64{1, 2}), "a")
50+
assert.Nil(t, err)
51+
_, err = stmt.ExecContext(ctx.Context(), "1", pq.Array([]int64{1, 2, 3}), "b")
52+
assert.Nil(t, err)
53+
// execute the migration
54+
txn, err := db.Begin()
55+
assert.Nil(t, err)
56+
assert.NotNil(t, txn)
57+
defer txn.Rollback()
58+
err = UpDropEventReferenceSHAPrevEvents(ctx.Context(), txn)
59+
assert.NoError(t, err)
60+
}

‎roomserver/storage/sqlite3/deltas/20230516154000_drop_reference_sha.go

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ import (
1818
"context"
1919
"database/sql"
2020
"fmt"
21+
22+
"github.com/lib/pq"
23+
"github.com/matrix-org/dendrite/internal"
24+
"github.com/matrix-org/util"
2125
)
2226

2327
func UpDropEventReferenceSHA(ctx context.Context, tx *sql.Tx) error {
@@ -52,8 +56,72 @@ func UpDropEventReferenceSHAPrevEvents(ctx context.Context, tx *sql.Tx) error {
5256
return fmt.Errorf("tx.ExecContext: %w", err)
5357
}
5458

59+
// figure out if there are duplicates
60+
dupeRows, err := tx.QueryContext(ctx, `SELECT previous_event_id FROM _roomserver_previous_events GROUP BY previous_event_id HAVING count(previous_event_id) > 1`)
61+
if err != nil {
62+
return fmt.Errorf("failed to query duplicate event ids")
63+
}
64+
defer internal.CloseAndLogIfError(ctx, dupeRows, "failed to close rows")
65+
66+
var prevEvents []string
67+
var prevEventID string
68+
for dupeRows.Next() {
69+
if err = dupeRows.Scan(&prevEventID); err != nil {
70+
return err
71+
}
72+
prevEvents = append(prevEvents, prevEventID)
73+
}
74+
if dupeRows.Err() != nil {
75+
return dupeRows.Err()
76+
}
77+
78+
// if we found duplicates, check if we can combine them, e.g. they are in the same room
79+
for _, dupeID := range prevEvents {
80+
var dupeNIDsRows *sql.Rows
81+
dupeNIDsRows, err = tx.QueryContext(ctx, `SELECT event_nids FROM _roomserver_previous_events WHERE previous_event_id = $1`, dupeID)
82+
if err != nil {
83+
return fmt.Errorf("failed to query duplicate event ids")
84+
}
85+
defer internal.CloseAndLogIfError(ctx, dupeNIDsRows, "failed to close rows")
86+
var dupeNIDs []int64
87+
for dupeNIDsRows.Next() {
88+
var nids pq.Int64Array
89+
if err = dupeNIDsRows.Scan(&nids); err != nil {
90+
return err
91+
}
92+
dupeNIDs = append(dupeNIDs, nids...)
93+
}
94+
95+
if dupeNIDsRows.Err() != nil {
96+
return dupeNIDsRows.Err()
97+
}
98+
// dedupe NIDs
99+
dupeNIDs = dupeNIDs[:util.SortAndUnique(nids(dupeNIDs))]
100+
// now that we have all NIDs, check which room they belong to
101+
var roomCount int
102+
err = tx.QueryRowContext(ctx, `SELECT count(distinct room_nid) FROM roomserver_events WHERE event_nid IN ($1)`, pq.Array(dupeNIDs)).Scan(&roomCount)
103+
if err != nil {
104+
return err
105+
}
106+
// if the events are from different rooms, that's bad and we can't continue
107+
if roomCount > 1 {
108+
return fmt.Errorf("detected events (%v) referenced for different rooms (%v)", dupeNIDs, roomCount)
109+
}
110+
// otherwise delete the dupes
111+
_, err = tx.ExecContext(ctx, "DELETE FROM _roomserver_previous_events WHERE previous_event_id = $1", dupeID)
112+
if err != nil {
113+
return fmt.Errorf("unable to delete duplicates: %w", err)
114+
}
115+
116+
// insert combined values
117+
_, err = tx.ExecContext(ctx, "INSERT INTO _roomserver_previous_events (previous_event_id, event_nids) VALUES ($1, $2)", dupeID, pq.Array(dupeNIDs))
118+
if err != nil {
119+
return fmt.Errorf("unable to insert new event NIDs: %w", err)
120+
}
121+
}
122+
55123
// move data
56-
if _, err := tx.ExecContext(ctx, `
124+
if _, err = tx.ExecContext(ctx, `
57125
INSERT
58126
INTO roomserver_previous_events (
59127
previous_event_id, event_nids
@@ -64,9 +132,15 @@ INSERT
64132
return fmt.Errorf("tx.ExecContext: %w", err)
65133
}
66134
// drop old table
67-
_, err := tx.ExecContext(ctx, `DROP TABLE _roomserver_previous_events;`)
135+
_, err = tx.ExecContext(ctx, `DROP TABLE _roomserver_previous_events;`)
68136
if err != nil {
69137
return fmt.Errorf("failed to execute upgrade: %w", err)
70138
}
71139
return nil
72140
}
141+
142+
type nids []int64
143+
144+
func (s nids) Len() int { return len(s) }
145+
func (s nids) Less(i, j int) bool { return s[i] < s[j] }
146+
func (s nids) Swap(i, j int) { s[i], s[j] = s[j], s[i] }

‎roomserver/storage/sqlite3/deltas/20230516154000_drop_reference_sha_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
package deltas
2+
3+
import (
4+
"testing"
5+
6+
"github.com/matrix-org/dendrite/internal/sqlutil"
7+
"github.com/matrix-org/dendrite/test"
8+
"github.com/matrix-org/dendrite/test/testrig"
9+
"github.com/stretchr/testify/assert"
10+
)
11+
12+
func TestUpDropEventReferenceSHAPrevEvents(t *testing.T) {
13+
14+
cfg, ctx, close := testrig.CreateConfig(t, test.DBTypeSQLite)
15+
defer close()
16+
17+
db, err := sqlutil.Open(&cfg.RoomServer.Database, sqlutil.NewExclusiveWriter())
18+
assert.Nil(t, err)
19+
assert.NotNil(t, db)
20+
defer db.Close()
21+
22+
// create the table in the old layout
23+
_, err = db.ExecContext(ctx.Context(), `
24+
CREATE TABLE IF NOT EXISTS roomserver_previous_events (
25+
previous_event_id TEXT NOT NULL,
26+
previous_reference_sha256 BLOB,
27+
event_nids TEXT NOT NULL,
28+
UNIQUE (previous_event_id, previous_reference_sha256)
29+
);`)
30+
assert.Nil(t, err)
31+
32+
// create the events table as well, slimmed down with one eventNID
33+
_, err = db.ExecContext(ctx.Context(), `
34+
CREATE TABLE IF NOT EXISTS roomserver_events (
35+
event_nid INTEGER PRIMARY KEY AUTOINCREMENT,
36+
room_nid INTEGER NOT NULL
37+
);
38+
39+
INSERT INTO roomserver_events (event_nid, room_nid) VALUES (1, 1)
40+
`)
41+
assert.Nil(t, err)
42+
43+
// insert duplicate prev events with different event_nids
44+
stmt, err := db.PrepareContext(ctx.Context(), `INSERT INTO roomserver_previous_events (previous_event_id, event_nids, previous_reference_sha256) VALUES ($1, $2, $3)`)
45+
assert.Nil(t, err)
46+
assert.NotNil(t, stmt)
47+
_, err = stmt.ExecContext(ctx.Context(), "1", "{1,2}", "a")
48+
assert.Nil(t, err)
49+
_, err = stmt.ExecContext(ctx.Context(), "1", "{1,2,3}", "b")
50+
assert.Nil(t, err)
51+
52+
// execute the migration
53+
txn, err := db.Begin()
54+
assert.Nil(t, err)
55+
assert.NotNil(t, txn)
56+
err = UpDropEventReferenceSHAPrevEvents(ctx.Context(), txn)
57+
defer txn.Rollback()
58+
assert.NoError(t, err)
59+
}

0 commit comments

Comments
 (0)