Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the cache eviction logic for stale entities more robust #12460

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 29 additions & 5 deletions cache/dynacache/dynacache.go
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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