lobsters / lobsters-ansible

Ansible playbook for lobste.rs
ISC License
79 stars 26 forks source link

nginx config for full-page caching #32

Closed pushcx closed 11 months ago

pushcx commented 6 years ago

I'd like to add actionpack-page_caching to cache full pages for logged-out users. The Rails side was straightforward, but the conditional might be too complex to express in nginx's config.

The sample nginx config says:

# Index HTML Files
if (-f $document_root/page_cache/$uri/index.html) {
  rewrite (.*) /page_cache/$1/index.html break;
}

# Other HTML Files
if (-f $document_root/page_cache/$uri.html) {
  rewrite (.*) /page_cache/$1.html break;
}

# All
if (-f $document_root/page_cache/$uri) {
  rewrite (.*) /page_cache/$1 break;
}

But we only want this to fire if the user doesn't have a lobster_trap or tag_filters cookie. Their absence can be detected with something like:

  set $use_file_cache "";
  if ($http_cookie ~* "lobster_trap") {
    set $use_file_cache "${skip_file_cache}S";
  }
  if ($http_cookie ~* "tag_filters") {
    set $use_file_cache "${skip_file_cache}F";
  }

But nginx doesn't allow nesting or combining ifs, so I don't know how to combine the conditionals above with $user_file_cache = SF. I can't see a similar way to combine conditionals and capture $1 for the rewrites. This also feels pretty clunky. Can anyone see how to combine these, or an alternate approach to only rewrite into the cached pages when the two cookies don't exist?

sean-public commented 6 years ago

Just do both if statements, setting part of a variable and then check if it has both parts afterwards.

Example.

pushcx commented 6 years ago

I'm aware of the strategy, I did it in my example above. Will it maintain backreferences from a previous conditional, or are they scoped?

sean-public commented 6 years ago

I don't think you understand: I meant that you can solve it by using the strategy again to check for SF && the path in question.

pushcx commented 6 years ago

I've got it caching and expiring the cache. Unfortunately, nginx isn't serving out of the cache. I don't know how to debug this, can anyone suggest methods or spot the bug in the commit? (@sean-public ?)

pushcx commented 6 years ago

I caught a likely bug that this should go inside the location / stanza, not above it.

@alanpost pointed out that Rails sets the lobster_trap cookie (its session cookie) every pageload, so that's not viable. To communicate to nginx that there isn't a logged-in user, I'll have to split out a separate cookie for maintaining login, unless someone else can think of a better approach.

Finally, even simpler testcases didn't work. I tried adding

   if ($http_cookie ~* "cargo=") {
    rewrite ^(.*)$ /cache/rss.rss break;
  }

and loading with curl --cookie "cargo=cult" https://lobste.rs/asdf but always got the generic 404 instead of the rss page. Alan tried this, same results:

  if ($cookie_cargo = "cult") {
    rewrite ^(.*)$ /cache/rss.rss break;
  }

The rewrites for http -> https and www -> no-www work, so it's not that all rewriting is broken.

jstoja commented 6 years ago

I quickly tried on my test environment with the last files that you commited (that are probably not up to date), but still, I don't understand clearly what the issue is. I tested this:

    # file-based full-page caching, bypass if user has cookies
    set $use_file_cache "";
    if ($http_cookie ~* "cargo") {
      set $use_file_cache "${use_file_cache}C";
    }
    if (-f $document_root/cache/$uri/index.html) {
      set $use_file_cache "${use_file_cache}I";
    }
    if ($use_file_cache = "CI") {
      rewrite (.*) /cache/$1/index.html break;
    }
    if (!-f $request_filename) {
      proxy_pass http://lobsters_unicorn_server;
      break;
    }

and get

$ curl --cookie "cargo=cult" localhost:8080/ 
<cached_page>
$ curl localhost:8080/ 
<uncached_page>

I probably don't get what is your real issue at the moment. So I cannot test if I can manage to make it work on my side. I only tried with the index, but it should be representative enough I think.

pushcx commented 6 years ago

Thank you for taking the time to experiment, it's really useful to hear. If that worked for you, then something's wrong on prod because several of my experiments should have worked. Either nginx isn't reloading (and systemctl is failing to stop + start) or something's off in our config that cookies aren't loading, an if isn't regexping, file perms are off, or something else fundamental is wrong.

jstoja commented 6 years ago

Would you be able to share (maybe in a separate branch) the configurations that you actually have for:

Maybe there are some issues in the error logs of unicorn and nginx and it's hard to see the relevant bits with the traffic of Lobsters. Checking those might help too. The only issue that I encoured was rather small, the nginx process hadn't the read rights of the cache directory. Checking the owner and mods of the directories/files of these + the user of nginx could be nice too. To be honest, I had to create/modify many things of this repo to have the application working in a new vm, so the configurations might diverge quite a lot.

pushcx commented 11 months ago

Fixed in c88660c and lobsters/lobsters@d806f7a6

Having always for add_header made debugging things a lot easier.