~sircmpwn/godocs.io

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
3 3

[PATCH] gddo-server: Invalidate Etag when subpackages & importers updated

Teddy Wing
Details
Message ID
<20210619124425.GA333@Aeris.local>
DKIM signature
permerror
Download raw message
Patch: +28 -9
Partially revert the Etag change in
be62deeb42eeb7a2a467b9751e34108d8e498047. That change removed
subpackages and importer count from the Etag hash. This meant that even
if subpackages changed, the old cached version would be loaded.

Packages with many subpackages (like `google.golang.org/api`) can take
time before they're fully fetched in the background. Once the package
was fetched with a partial list of subpackages, none of the rest of the
subpackages would appear on subsequent page reloads due to the Etag
cache, even though more subpackages had been saved into the database.

Restore some of the prior Etag behaviour, including the subpackages and
importer count in the hash. This invalidates the cache when subpackages
are added so they can be seen on a page refresh.

I'm unclear on what the `importerCount` limit at 8 is for, but figured
I'd reuse the code from just before
be62deeb42eeb7a2a467b9751e34108d8e498047.
---
Here's what it looks like before:

https://teddywing.com/files/patches/gddo/etag/Before.webm

and after:

https://teddywing.com/files/patches/gddo/etag/After.webm

 gddo-server/http.go | 37 ++++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/gddo-server/http.go b/gddo-server/http.go
index e4960fe..3a1e167 100644
--- a/gddo-server/http.go
+++ b/gddo-server/http.go
@@ -12,6 +12,7 @@ import (
	"net/http"
	"path/filepath"
	"runtime/debug"
	"strconv"
	"strings"
	"time"

@@ -64,11 +65,29 @@ func (s *Server) HTTPHandler() (http.Handler, error) {
}

// httpEtag returns the package entity tag used in HTTP transactions.
func (s *Server) httpEtag(importPath, version string, flashMessages []flashMessage) string {
func (s *Server) httpEtag(
	pkg *database.Package,
	subpkgs []database.Package,
	importerCount int,
	flashMessages []flashMessage,
) string {
	b := make([]byte, 0, 128)
	b = append(b, importPath...)
	b = append(b, pkg.ImportPath...)
	b = append(b, 0)
	b = append(b, version...)
	b = append(b, pkg.Version...)

	if importerCount >= 8 {
		importerCount = 8
	}

	b = strconv.AppendInt(b, int64(importerCount), 16)
	for _, subpkg := range subpkgs {
		b = append(b, 0)
		b = append(b, subpkg.ImportPath...)
		b = append(b, 0)
		b = append(b, subpkg.Synopsis...)
	}

	for _, m := range flashMessages {
		b = append(b, 0)
		b = append(b, m.ID...)
@@ -192,12 +211,6 @@ func (s *Server) servePackage(resp http.ResponseWriter, req *http.Request) error
		return nil

	default:
		etag := s.httpEtag(pkg.ImportPath, pkg.Version, flashMessages)
		status := http.StatusOK
		if req.Header.Get("If-None-Match") == etag {
			status = http.StatusNotModified
		}

		importCount, err := s.db.ImportCount(req.Context(), importPath)
		if err != nil {
			return err
@@ -210,6 +223,12 @@ func (s *Server) servePackage(resp http.ResponseWriter, req *http.Request) error
		}
		tctx.Package.SubPackages = subpkgs

		etag := s.httpEtag(pkg, subpkgs, importCount, flashMessages)
		status := http.StatusOK
		if req.Header.Get("If-None-Match") == etag {
			status = http.StatusNotModified
		}

		resp.Header().Set("Etag", etag)
		return s.templates.ExecuteHTML(resp, "doc.html", status, &tctx)
	}
-- 
2.27.0
Details
Message ID
<CC7OMXJP36TL.L2ZOZ4NX07K3@taiga>
In-Reply-To
<20210619124425.GA333@Aeris.local> (view parent)
DKIM signature
fail
Download raw message
DKIM signature: fail
Thanks!

To git@git.sr.ht:~sircmpwn/gddo
   df74f2d..5909268  master -> master
Details
Message ID
<CC7R4SJQLIAD.1RW6N8EV4RT9J@nitro>
In-Reply-To
<20210619124425.GA333@Aeris.local> (view parent)
DKIM signature
pass
Download raw message
On Sat Jun 19, 2021 at 8:44 AM EDT, Teddy Wing wrote:
> I'm unclear on what the `importerCount` limit at 8 is for, but figured
> I'd reuse the code from just before
> be62deeb42eeb7a2a467b9751e34108d8e498047.

I'm not sure what that is for either. Perhaps it can be removed?
Teddy Wing
Details
Message ID
<20210620150249.GB95427@Aeris.local>
In-Reply-To
<CC7R4SJQLIAD.1RW6N8EV4RT9J@nitro> (view parent)
DKIM signature
permerror
Download raw message
On Jun 19, 2021, at 12:56 PM -0400, Adnan Maolood <me@adnano.co> wrote:
>On Sat Jun 19, 2021 at 8:44 AM EDT, Teddy Wing wrote:
>> I'm unclear on what the `importerCount` limit at 8 is for, but figured
>> I'd reuse the code from just before
>> be62deeb42eeb7a2a467b9751e34108d8e498047.
>
>I'm not sure what that is for either. Perhaps it can be removed?

Removing the limit seems reasonable to me.
Reply to thread Export thread (mbox)