Skip to content

Commit

Permalink
Make the cache eviction logic for stale entities more robust
Browse files Browse the repository at this point in the history
Fixes #12458
  • Loading branch information
bep committed May 4, 2024
1 parent 68e9532 commit 503d209
Show file tree
Hide file tree
Showing 12 changed files with 157 additions and 55 deletions.
34 changes: 29 additions & 5 deletions cache/dynacache/dynacache.go
Expand Up @@ -385,13 +385,37 @@ type Partition[K comparable, V any] struct {

// GetOrCreate gets or creates a value for the given key.
func (p *Partition[K, V]) GetOrCreate(key K, create func(key K) (V, error)) (V, error) {
v, err := p.doGetOrCreate(key, create)
if err != nil {
return p.zero, err
}
if resource.StaleVersion(v) > 0 {
p.c.Delete(key)
return p.doGetOrCreate(key, create)
}
return v, err
}

func (p *Partition[K, V]) doGetOrCreate(key K, create func(key K) (V, error)) (V, error) {
v, _, err := p.c.GetOrCreate(key, create)
return v, err
}

func (p *Partition[K, V]) GetOrCreateWitTimeout(key K, duration time.Duration, create func(key K) (V, error)) (V, error) {
v, err := p.doGetOrCreateWitTimeout(key, duration, create)
if err != nil {
return p.zero, err
}
if resource.StaleVersion(v) > 0 {
p.c.Delete(key)
return p.doGetOrCreateWitTimeout(key, duration, create)
}
return v, err
}

// GetOrCreateWitTimeout gets or creates a value for the given key and times out if the create function
// takes too long.
func (p *Partition[K, V]) GetOrCreateWitTimeout(key K, duration time.Duration, create func(key K) (V, error)) (V, error) {
func (p *Partition[K, V]) doGetOrCreateWitTimeout(key K, duration time.Duration, create func(key K) (V, error)) (V, error) {
resultch := make(chan V, 1)
errch := make(chan error, 1)

Expand Down Expand Up @@ -448,7 +472,7 @@ func (p *Partition[K, V]) clearOnRebuild(changeset ...identity.Identity) {

shouldDelete := func(key K, v V) bool {
// We always clear elements marked as stale.
if resource.IsStaleAny(v) {
if resource.StaleVersion(v) > 0 {
return true
}

Expand Down Expand Up @@ -503,8 +527,8 @@ func (p *Partition[K, V]) Keys() []K {

func (p *Partition[K, V]) clearStale() {
p.c.DeleteFunc(func(key K, v V) bool {
isStale := resource.IsStaleAny(v)
if isStale {
staleVersion := resource.StaleVersion(v)
if staleVersion > 0 {
p.trace.Log(
logg.StringFunc(
func() string {
Expand All @@ -514,7 +538,7 @@ func (p *Partition[K, V]) clearStale() {
)
}

return isStale
return staleVersion > 0
})
}

Expand Down
12 changes: 6 additions & 6 deletions cache/dynacache/dynacache_test.go
Expand Up @@ -29,12 +29,12 @@ var (
)

type testItem struct {
name string
isStale bool
name string
staleVersion uint32
}

func (t testItem) IsStale() bool {
return t.isStale
func (t testItem) StaleVersion() uint32 {
return t.staleVersion
}

func (t testItem) IdentifierBase() string {
Expand Down Expand Up @@ -109,7 +109,7 @@ func newTestCache(t *testing.T) *Cache {

p2.GetOrCreate("clearBecauseStale", func(string) (testItem, error) {
return testItem{
isStale: true,
staleVersion: 32,
}, nil
})

Expand All @@ -121,7 +121,7 @@ func newTestCache(t *testing.T) *Cache {

p2.GetOrCreate("clearNever", func(string) (testItem, error) {
return testItem{
isStale: false,
staleVersion: 0,
}, nil
})

Expand Down
12 changes: 7 additions & 5 deletions hugolib/content_map_page.go
Expand Up @@ -824,10 +824,10 @@ func (s *contentNodeShifter) Insert(old, new contentNodeI) contentNodeI {
if !ok {
panic(fmt.Sprintf("unknown type %T", new))
}
if newp != old {
resource.MarkStale(old)
}
if vv.s.languagei == newp.s.languagei {
if newp != old {
resource.MarkStale(old)
}
return new
}
is := make(contentNodeIs, s.numLanguages)
Expand All @@ -843,7 +843,6 @@ func (s *contentNodeShifter) Insert(old, new contentNodeI) contentNodeI {
if oldp != newp {
resource.MarkStale(oldp)
}

vv[newp.s.languagei] = new
return vv
case *resourceSource:
Expand All @@ -852,6 +851,9 @@ func (s *contentNodeShifter) Insert(old, new contentNodeI) contentNodeI {
panic(fmt.Sprintf("unknown type %T", new))
}
if vv.LangIndex() == newp.LangIndex() {
if vv != newp {
resource.MarkStale(vv)
}
return new
}
rs := make(resourceSources, s.numLanguages)
Expand Down Expand Up @@ -1064,7 +1066,7 @@ func (h *HugoSites) resolveAndClearStateForIdentities(
)

for _, id := range changes {
if staler, ok := id.(resource.Staler); ok && !staler.IsStale() {
if staler, ok := id.(resource.Staler); ok {
var msgDetail string
if p, ok := id.(*pageState); ok && p.File() != nil {
msgDetail = fmt.Sprintf(" (%s)", p.File().Filename())
Expand Down
29 changes: 18 additions & 11 deletions hugolib/page__content.go
Expand Up @@ -418,6 +418,8 @@ func (c *cachedContent) mustSource() []byte {

func (c *contentParseInfo) contentSource(s resource.StaleInfo) ([]byte, error) {
key := c.sourceKey
versionv := s.StaleVersion()

v, err := c.h.cacheContentSource.GetOrCreate(key, func(string) (*resources.StaleValue[[]byte], error) {
b, err := c.readSourceAll()
if err != nil {
Expand All @@ -426,8 +428,8 @@ func (c *contentParseInfo) contentSource(s resource.StaleInfo) ([]byte, error) {

return &resources.StaleValue[[]byte]{
Value: b,
IsStaleFunc: func() bool {
return s.IsStale()
StaleVersionFunc: func() uint32 {
return s.StaleVersion() - versionv
},
}, nil
})
Expand Down Expand Up @@ -487,7 +489,7 @@ type contentPlainPlainWords struct {
func (c *cachedContent) contentRendered(ctx context.Context, cp *pageContentOutput) (contentSummary, error) {
ctx = tpl.Context.DependencyScope.Set(ctx, pageDependencyScopeGlobal)
key := c.pi.sourceKey + "/" + cp.po.f.Name
versionv := cp.contentRenderedVersion
versionv := c.version(cp)

v, err := c.pm.cacheContentRendered.GetOrCreate(key, func(string) (*resources.StaleValue[contentSummary], error) {
cp.po.p.s.Log.Trace(logg.StringFunc(func() string {
Expand All @@ -504,8 +506,8 @@ func (c *cachedContent) contentRendered(ctx context.Context, cp *pageContentOutp
}

rs := &resources.StaleValue[contentSummary]{
IsStaleFunc: func() bool {
return c.IsStale() || cp.contentRenderedVersion != versionv
StaleVersionFunc: func() uint32 {
return c.version(cp) - versionv
},
}

Expand Down Expand Up @@ -607,7 +609,7 @@ var setGetContentCallbackInContext = hcontext.NewContextDispatcher[func(*pageCon

func (c *cachedContent) contentToC(ctx context.Context, cp *pageContentOutput) (contentTableOfContents, error) {
key := c.pi.sourceKey + "/" + cp.po.f.Name
versionv := cp.contentRenderedVersion
versionv := c.version(cp)

v, err := c.pm.contentTableOfContents.GetOrCreate(key, func(string) (*resources.StaleValue[contentTableOfContents], error) {
source, err := c.pi.contentSource(c)
Expand Down Expand Up @@ -713,8 +715,8 @@ func (c *cachedContent) contentToC(ctx context.Context, cp *pageContentOutput) (

return &resources.StaleValue[contentTableOfContents]{
Value: ct,
IsStaleFunc: func() bool {
return c.IsStale() || cp.contentRenderedVersion != versionv
StaleVersionFunc: func() uint32 {
return c.version(cp) - versionv
},
}, nil
})
Expand All @@ -725,16 +727,21 @@ func (c *cachedContent) contentToC(ctx context.Context, cp *pageContentOutput) (
return v.Value, nil
}

func (c *cachedContent) version(cp *pageContentOutput) uint32 {
// Both of these gets incremented on change.
return c.StaleVersion() + cp.contentRenderedVersion
}

func (c *cachedContent) contentPlain(ctx context.Context, cp *pageContentOutput) (contentPlainPlainWords, error) {
key := c.pi.sourceKey + "/" + cp.po.f.Name

versionv := cp.contentRenderedVersion
versionv := c.version(cp)

v, err := c.pm.cacheContentPlain.GetOrCreateWitTimeout(key, cp.po.p.s.Conf.Timeout(), func(string) (*resources.StaleValue[contentPlainPlainWords], error) {
var result contentPlainPlainWords
rs := &resources.StaleValue[contentPlainPlainWords]{
IsStaleFunc: func() bool {
return c.IsStale() || cp.contentRenderedVersion != versionv
StaleVersionFunc: func() uint32 {
return c.version(cp) - versionv
},
}

Expand Down
4 changes: 2 additions & 2 deletions hugolib/page__per_output.go
Expand Up @@ -89,8 +89,8 @@ type pageContentOutput struct {
// typically included with .RenderShortcodes.
otherOutputs map[uint64]*pageContentOutput

contentRenderedVersion int // Incremented on reset.
contentRendered bool // Set on content render.
contentRenderedVersion uint32 // Incremented on reset.
contentRendered bool // Set on content render.

// Renders Markdown hooks.
renderHooks *renderHooks
Expand Down
28 changes: 23 additions & 5 deletions hugolib/rebuild_test.go
Expand Up @@ -53,6 +53,11 @@ title: "Home"
Home Content.
-- content/hometext.txt --
Home Text Content.
-- content/myothersection/myothersectionpage.md --
---
title: "myothersectionpage"
---
myothersectionpage Content.
-- layouts/_default/single.html --
Single: {{ .Title }}|{{ .Content }}$
Resources: {{ range $i, $e := .Resources }}{{ $i }}:{{ .RelPermalink }}|{{ .Content }}|{{ end }}$
Expand Down Expand Up @@ -135,8 +140,8 @@ func TestRebuildRenameTextFileInLeafBundle(t *testing.T) {

b.RenameFile("content/mysection/mysectionbundle/mysectionbundletext.txt", "content/mysection/mysectionbundle/mysectionbundletext2.txt").Build()
b.AssertFileContent("public/mysection/mysectionbundle/index.html", "mysectionbundletext2", "My Section Bundle Text 2 Content.", "Len Resources: 2|")
b.AssertRenderCountPage(3)
b.AssertRenderCountContent(3)
b.AssertRenderCountPage(5)
b.AssertRenderCountContent(6)
})
}

Expand All @@ -147,6 +152,19 @@ func TestRebuilEditContentFileInLeafBundle(t *testing.T) {
b.AssertFileContent("public/mysection/mysectionbundle/index.html", "My Section Bundle Content Content Edited.")
}

func TestRebuilEditContentFileThenAnother(t *testing.T) {
b := TestRunning(t, rebuildFilesSimple)
b.EditFileReplaceAll("content/mysection/mysectionbundle/mysectionbundlecontent.md", "Content Content.", "Content Content Edited.").Build()
b.AssertFileContent("public/mysection/mysectionbundle/index.html", "My Section Bundle Content Content Edited.")
b.AssertRenderCountPage(1)
b.AssertRenderCountContent(2)

b.EditFileReplaceAll("content/myothersection/myothersectionpage.md", "myothersectionpage Content.", "myothersectionpage Content Edited.").Build()
b.AssertFileContent("public/myothersection/myothersectionpage/index.html", "myothersectionpage Content Edited")
b.AssertRenderCountPage(1)
b.AssertRenderCountContent(1)
}

func TestRebuildRenameTextFileInBranchBundle(t *testing.T) {
b := TestRunning(t, rebuildFilesSimple)
b.AssertFileContent("public/mysection/index.html", "My Section")
Expand All @@ -163,7 +181,7 @@ func TestRebuildRenameTextFileInHomeBundle(t *testing.T) {

b.RenameFile("content/hometext.txt", "content/hometext2.txt").Build()
b.AssertFileContent("public/index.html", "hometext2", "Home Text Content.")
b.AssertRenderCountPage(2)
b.AssertRenderCountPage(3)
}

func TestRebuildRenameDirectoryWithLeafBundle(t *testing.T) {
Expand All @@ -179,7 +197,7 @@ func TestRebuildRenameDirectoryWithBranchBundle(t *testing.T) {
b.AssertFileContent("public/mysectionrenamed/index.html", "My Section")
b.AssertFileContent("public/mysectionrenamed/mysectionbundle/index.html", "My Section Bundle")
b.AssertFileContent("public/mysectionrenamed/mysectionbundle/mysectionbundletext.txt", "My Section Bundle Text 2 Content.")
b.AssertRenderCountPage(2)
b.AssertRenderCountPage(3)
}

func TestRebuildRenameDirectoryWithRegularPageUsedInHome(t *testing.T) {
Expand Down Expand Up @@ -278,7 +296,7 @@ func TestRebuildRenameDirectoryWithBranchBundleFastRender(t *testing.T) {
b.AssertFileContent("public/mysectionrenamed/index.html", "My Section")
b.AssertFileContent("public/mysectionrenamed/mysectionbundle/index.html", "My Section Bundle")
b.AssertFileContent("public/mysectionrenamed/mysectionbundle/mysectionbundletext.txt", "My Section Bundle Text 2 Content.")
b.AssertRenderCountPage(2)
b.AssertRenderCountPage(3)
}

func TestRebuilErrorRecovery(t *testing.T) {
Expand Down
37 changes: 37 additions & 0 deletions hugolib/rendershortcodes_test.go
Expand Up @@ -201,6 +201,43 @@ Myshort Original.
b.AssertFileContent("public/p1/index.html", "Edited")
}

func TestRenderShortcodesEditSectionContentWithShortcodeInIncludedPageIssue12458(t *testing.T) {
t.Parallel()

files := `
-- hugo.toml --
disableLiveReload = true
disableKinds = ["home", "taxonomy", "term", "rss", "sitemap", "robotsTXT", "404"]
-- content/mysection/_index.md --
---
title: "My Section"
---
## p1-h1
{{% include "p2" %}}
-- content/mysection/p2.md --
---
title: "p2"
---
### Original
{{% myshort %}}
-- layouts/shortcodes/include.html --
{{ $p := .Page.GetPage (.Get 0) }}
{{ $p.RenderShortcodes }}
-- layouts/shortcodes/myshort.html --
Myshort Original.
-- layouts/_default/list.html --
{{ .Content }}
`
b := TestRunning(t, files)

b.AssertFileContent("public/mysection/index.html", "p1-h1")
b.EditFileReplaceAll("content/mysection/_index.md", "p1-h1", "p1-h1 Edited").Build()
b.AssertFileContent("public/mysection/index.html", "p1-h1 Edited")
}

func TestRenderShortcodesNestedPageContextIssue12356(t *testing.T) {
t.Parallel()

Expand Down
2 changes: 1 addition & 1 deletion hugolib/site_benchmark_new_test.go
Expand Up @@ -487,7 +487,7 @@ Edited!!`, p.Title()))

// We currently rebuild all the language versions of the same content file.
// We could probably optimize that case, but it's not trivial.
b.Assert(int(counters.contentRenderCounter.Load()), qt.Equals, 33)
b.Assert(int(counters.contentRenderCounter.Load()), qt.Equals, 4)
b.AssertFileContent("public"+p.RelPermalink()+"index.html", "Edited!!")
}

Expand Down

0 comments on commit 503d209

Please sign in to comment.