Implement HTTP proxy for primary redirection & consistency by benbjohnson · Pull Request #271 · superfly/litefs

benbjohnson · 2023-02-01T22:04:17Z

This pull request implements a thin HTTP proxy that can run in front of an application and automatically handle primary redirection & TXID consistency tracking. This works for applications that use read-only GET requests and only use a single SQLite database.

It implements the following:

For non-GET requests on a replica, a fly-replay is returned to automatically forward to the primary.
For non-GET requests on the primary, a __txid cookie is set with the current database TXID.
For GET requests on a replica, it will wait until the database catches up to the TXID stored in the cookie.

Fixes #269

Configuration

To enable the proxy, you need to set several fields in the proxy section of the config:

proxy:
  # Bind address for the proxy to listen on.
  # This should match your "services.internal_port" in your fly.toml
  addr: ":8080"

  # Hostport of your application. You'll need to change the port on your
  # application so Fly only talks to it through the proxy.
  target: "localhost:8081"

  # Filename of the SQLite database you want to use for TXID tracking.
  db: "my.db"

  # If true, this enables verbose debug logging for the proxy.
  debug: false

benbjohnson · 2023-02-01T22:06:23Z

@kentcdodds I have tests around the primary redirection and that works but I'm going to fire it up on a Fly.io test app and make sure the TXID consistency tracking is working on replicas. It's hard to test that locally since it happens really fast in tests.

This is still very much a work in progress! Please don't deploy to production! ;)

benbjohnson · 2023-02-02T17:15:54Z

@kentcdodds I did some more testing on this proxy this morning and it seems to be working pretty well. I'm going to merge this in and if you have any issues then we can address those separately.

Also, note that if you upgrade to this PR then you'll get compression (#132) as well and you won't be able to easily rollback to a v0.3.0 version.

kentcdodds · 2023-02-02T17:22:36Z

http/proxy_server.go

+
+	// No TXID or we couldn't parse it. Just send to the target.
+	if txid == 0 {
+		s.logf("proxy: %s %s: no client txid, proxying to target", r.Method, r.URL.Path)


Does this log show up all the time or can I turn it on/off? I have a feeling we'd see this a lot 🤔

The logf() messages only show up if you set proxy.debug. Otherwise they're no-ops.

Excellent. Thank you! I'm going to test this out today.

kentcdodds · 2023-02-02T17:33:18Z

This is great!

All that is good to know!

To be clear, I can have multiple sqlite databases, but I can only specify one of them as the one for txid tracking right? That's fine for my use cases, I just wanted to be sure.

This is a pretty elegant solution I think.

benbjohnson · 2023-02-02T17:43:08Z

Yes, that’s correct. Multiple databases is fine. I had thought about allowing a list of databases to use for TXID tracking but that seemed overly complicated.

kentcdodds · 2023-02-02T17:58:10Z

Worked in a simple case, now I'm trying it on my own site to see if it works for a complex case.

One thing I want to double check is that it doesn't override the set-cookie header that the target server already set.

kentcdodds · 2023-02-02T17:59:18Z

http/proxy_server.go

+		if db := s.store.DB(s.DBName); db != nil {
+			pos := db.Pos()
+			s.logf("proxy: %s %s: setting txid cookie to %s", r.Method, r.URL.Path, ltx.FormatTXID(pos.TXID))
+			http.SetCookie(w, &http.Cookie{


Does this override an existing header on the response or does it append? We'll need it to append.

It appends the cookie so it shouldn't affect application cookies (unless they're also named __txid): https://cs.opensource.google/go/go/+/refs/tags/go1.20:src/net/http/cookie.go;l=171

Just to clarify, Header().Add() appends. If it overrode it then it would be Header().Set().

I don't see a Header().Add() or a Header().Set() with regard to this cookie 🤔 But if you say we're good then I'll believe you 😅

The http.SetCookie() calls w.Header().Add(). Sorry, I should have been more clear.

func SetCookie(w ResponseWriter, cookie *Cookie) { if v := cookie.String(); v != "" { w.Header().Add("Set-Cookie", v) } }

kentcdodds · 2023-02-02T20:46:12Z

Should we ever delete the txid cookie? Seems reasonable to do that just in case they get to a high cookie and then we deploy a new instance of our app where the txid starts over. If we never delete the cookie then users would always get directed to primary until the new db caught up.

benbjohnson · 2023-02-02T22:02:45Z

Should we ever delete the txid cookie?

I added the expiration in case someone needed to revert their database snapshot to an older version. For example, if you revert to a snapshot from yesterday then clients that saw a TXID just before the reversion could be waiting 24 hours for enough writes to come through to catch up.

I arbitrarily chose 5 minutes since that's a decently long time for a node to be disconnected from the primary.

kentcdodds · 2023-02-02T22:46:38Z

That makes perfect sense 👍 Thanks!

ccll · 2023-04-07T09:37:55Z

Hello, the proxy feature is very promising, but I got some issue when trying it out with 30X redirections.

Currently the 'set-cookie' header seems not setting the 'Path=' attribute, so it defaults to the path segment in current URL.
Say first I issue a write request with POST /write/to/database, the response is a redirection to another read request URL GET /redirect/to/here, I can see that __txid cookie is correctly set in the first response, but it'll never send in the second request cause the URL path is different.

I'm not an expert on this, I think the proxy should set __txid cookie with Path=/ in this case, so any GET requests after the non-GET request will correctly send the cookie, right?

benbjohnson · 2023-04-08T00:32:07Z

@ccll Thanks for reporting that. Yes, it should be setting the cookie path. I added a fix on #304.

benbjohnson force-pushed the proxy branch 3 times, most recently from de414c3 to bdb1c6e Compare February 2, 2023 16:47

Implement thin http proxy

8441f7a

benbjohnson force-pushed the proxy branch from bdb1c6e to 8441f7a Compare February 2, 2023 16:53

benbjohnson marked this pull request as ready for review February 2, 2023 16:54

benbjohnson merged commit ec7ccd5 into main Feb 2, 2023

benbjohnson deleted the proxy branch February 2, 2023 17:14

kentcdodds reviewed Feb 2, 2023

View reviewed changes

benbjohnson added this to the v0.4.0 milestone Mar 3, 2023

benbjohnson mentioned this pull request Apr 2, 2023

Litefs secondary replication position #301

Closed

benbjohnson mentioned this pull request Apr 8, 2023

Set path for proxy server cookie #304

Merged

Conversation

benbjohnson commented Feb 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Configuration

Uh oh!

benbjohnson commented Feb 1, 2023

Uh oh!

benbjohnson commented Feb 2, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kentcdodds commented Feb 2, 2023

Uh oh!

benbjohnson commented Feb 2, 2023

Uh oh!

kentcdodds commented Feb 2, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kentcdodds commented Feb 2, 2023

Uh oh!

benbjohnson commented Feb 2, 2023

Uh oh!

kentcdodds commented Feb 2, 2023

Uh oh!

ccll commented Apr 7, 2023

Uh oh!

benbjohnson commented Apr 8, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

benbjohnson commented Feb 1, 2023 •

edited

Loading