fix: rebuilds collect under one lock and keep moved lastmods
A rebuild takes the transaction-scoped advisory lock golem15.sitemap.rebuild before it reads the settings and collects, and applies in the same transaction, so a second rebuild collects only after the first has committed. apply no longer writes the lastmod of a content row it did not create, so a lastmod RecordSaved moved meanwhile stays.
This commit is contained in:
@@ -114,14 +114,21 @@ type collected struct {
|
||||
|
||||
// Rebuild reads every content type's published records (signpost Records,
|
||||
// D-93) and the additional addresses, writes them into the slot table and
|
||||
// numbers the files, in one transaction that holds the settings row, so
|
||||
// two rebuilds never interleave. A record that is gone or unpublished
|
||||
// leaves its rows in the table with a reason, as a hole; only a changed
|
||||
// file size renumbers the files (D-30). In production, with IndexNow on,
|
||||
// the added, changed and removed addresses are queued for IndexNow after
|
||||
// the commit (NotifyIndexNow), with the rows the record listeners marked
|
||||
// numbers the files. A record that is gone or unpublished leaves its rows
|
||||
// in the table with a reason, as a hole; only a changed file size
|
||||
// renumbers the files (D-30). In production, with IndexNow on, the added,
|
||||
// changed and removed addresses are queued for IndexNow after the commit
|
||||
// (NotifyIndexNow), with the rows the record listeners marked
|
||||
// (indexnow_pending_at): a deleted record's addresses as removed, an
|
||||
// edited record's listed addresses as changed.
|
||||
//
|
||||
// Two rebuilds never interleave (L2 WR-02): one transaction takes the
|
||||
// transaction-scoped advisory lock golem15.sitemap.rebuild first, then
|
||||
// reads the settings and the type settings, collects the records, locks
|
||||
// the settings row and applies, so a second rebuild collects only after
|
||||
// the first has committed and an older snapshot never overwrites a newer
|
||||
// one. A rebuild never writes the lastmod of a content row it did not
|
||||
// create (WR-03), so a lastmod that RecordSaved moved meanwhile stays.
|
||||
func Rebuild(ctx context.Context, app *backpack.App) (RebuildResult, error) {
|
||||
var res RebuildResult
|
||||
if app == nil {
|
||||
@@ -132,22 +139,25 @@ func Rebuild(ctx context.Context, app *backpack.App) (RebuildResult, error) {
|
||||
return res, ErrNoDatabase
|
||||
}
|
||||
db = db.WithContext(ctx)
|
||||
settings, err := LoadSettings(db)
|
||||
if err != nil {
|
||||
return res, err
|
||||
}
|
||||
types, err := LoadTypeSettings(db)
|
||||
if err != nil {
|
||||
return res, err
|
||||
}
|
||||
reg, _ := app.Lookup[*signpost.Registry]()
|
||||
filter, _ := app.Lookup[contract.Filter]()
|
||||
site := SiteURL(app, settings)
|
||||
found, err := collect(ctx, reg, filter, settings, types, site)
|
||||
if err != nil {
|
||||
return res, err
|
||||
}
|
||||
err = db.Transaction(func(tx *gorm.DB) error {
|
||||
err := db.Transaction(func(tx *gorm.DB) error {
|
||||
if err := tx.Exec(`SELECT pg_advisory_xact_lock(hashtext('golem15.sitemap.rebuild'))`).Error; err != nil {
|
||||
return err
|
||||
}
|
||||
settings, err := LoadSettings(tx)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
types, err := LoadTypeSettings(tx)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
site := SiteURL(app, settings)
|
||||
found, err := collect(ctx, reg, filter, settings, types, site)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if err := tx.Exec(`SELECT id FROM golem15_sitemap_settings WHERE id = 1 FOR UPDATE`).Error; err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -155,7 +165,7 @@ func Rebuild(ctx context.Context, app *backpack.App) (RebuildResult, error) {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
res, err = apply(tx, found, locked, site)
|
||||
res, err = apply(ctx, tx, found, locked, site)
|
||||
return err
|
||||
})
|
||||
if err != nil {
|
||||
@@ -169,6 +179,16 @@ func Rebuild(ctx context.Context, app *backpack.App) (RebuildResult, error) {
|
||||
return res, nil
|
||||
}
|
||||
|
||||
// afterSlotReadKey carries the WithAfterSlotRead hook.
|
||||
type afterSlotReadKey struct{}
|
||||
|
||||
// WithAfterSlotRead returns ctx with fn, which a rebuild on ctx calls right
|
||||
// after apply has read the slot rows and before it writes them: a test
|
||||
// seam for what happens concurrently (nil, the default, calls nothing).
|
||||
func WithAfterSlotRead(ctx context.Context, fn func()) context.Context {
|
||||
return context.WithValue(ctx, afterSlotReadKey{}, fn)
|
||||
}
|
||||
|
||||
// collect walks the content types page by page and adds the additional
|
||||
// addresses. The records of a type switched off in the settings are kept
|
||||
// in the table with the reason contract.ReasonType, so the settings screen
|
||||
@@ -411,12 +431,15 @@ func setAlternates(rows []wanted, defaultLocale string) {
|
||||
|
||||
// apply writes the found addresses into the slot table and numbers the
|
||||
// files, inside the rebuild's transaction.
|
||||
func apply(tx *gorm.DB, found collected, settings models.Settings, site string) (RebuildResult, error) {
|
||||
func apply(ctx context.Context, tx *gorm.DB, found collected, settings models.Settings, site string) (RebuildResult, error) {
|
||||
var res RebuildResult
|
||||
var existing []models.URL
|
||||
if err := tx.Order("id ASC").Find(&existing).Error; err != nil {
|
||||
return res, err
|
||||
}
|
||||
if fn, _ := ctx.Value(afterSlotReadKey{}).(func()); fn != nil {
|
||||
fn()
|
||||
}
|
||||
now := time.Now().UTC().Truncate(time.Microsecond)
|
||||
byKey := make(map[slotKey]*models.URL, len(existing))
|
||||
for i := range existing {
|
||||
@@ -582,12 +605,11 @@ func apply(tx *gorm.DB, found collected, settings models.Settings, site string)
|
||||
for id := range dirty {
|
||||
r := rowsByID[id]
|
||||
r.UpdatedAt = now
|
||||
if err := tx.Model(&models.URL{}).Where("id = ?", id).Updates(map[string]any{
|
||||
values := map[string]any{
|
||||
"grp": r.Grp,
|
||||
"url": r.URL,
|
||||
"title": r.Title,
|
||||
"file_no": r.FileNo,
|
||||
"lastmod": r.LastMod,
|
||||
"alternates": r.Alternates,
|
||||
"images": r.Images,
|
||||
"image_file_no": r.ImageFileNo,
|
||||
@@ -596,7 +618,14 @@ func apply(tx *gorm.DB, found collected, settings models.Settings, site string)
|
||||
"parent_refs": r.ParentRefs,
|
||||
"excluded_reason": r.ExcludedReason,
|
||||
"updated_at": r.UpdatedAt,
|
||||
}).Error; err != nil {
|
||||
}
|
||||
// Only an additional address takes its lastmod from the rebuild; a
|
||||
// content row's lastmod is RecordSaved's, never written back from
|
||||
// this rebuild's read (WR-03).
|
||||
if r.TypeKey == models.ExtraTypeKey {
|
||||
values["lastmod"] = r.LastMod
|
||||
}
|
||||
if err := tx.Model(&models.URL{}).Where("id = ?", id).Updates(values).Error; err != nil {
|
||||
return res, err
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user