We're running Tome/Static for a client project. There's an hourly build that happens, that then pushes out to an Azure blob. The entire process works fine, it's just that the resulting static directory has inconsistent rules.

On a fresh build (no static directory exists, empty cache table): anywhere from 620-850 files. On subsequent builds, it then will do the same thing. It'll either create the missing files, or will delete existing files (sometimes a few, sometimes a lot). I still can't figure out why there is such a large discrepancy between current runs of Tome though.

I did make a couple changes to the static cache code for CID generation: forcing lowercase and trimming any trailing slashes. These two issues were causing multiple cache entries to appear for the same destination, and those destinations were then getting deleted and not regenerated on subsequent Tome runs.

We've even gone so far as doing a full build (running drush tome:super-cache-rebuild before Tome), same problem occurs. I've even disabled caching on Drupal (setting multiple cache bins to null backend in settings), still same problem.

I cannot figure out why these files are getting deleted during Tome runs, or why they aren't generated on the initial run but are during subsequent runs.

Any ideas of things to try or look into?

Comments

jpontani created an issue. See original summary.

samuel.mortenson’s picture

StatusFileSize
new1.77 KB

Hi @jpontani - thanks for trying out Tome and for using Tome Static Super Cache, you may be the first person to use that!

My gut feeling is that cache is being invalidated too often on your site, causing many pages to be marked for deletion. To understand this better, I'll note that there are only two cases where Tome will delete a static file:

1. When the cache is empty, \Drupal\tome_static\StaticGenerator::prepareStaticDirectory will delete all exported files. When using Tome Static Super Cache, you have to explicitly clear Tome Static cache to have this happen, "drush cr" won't actually delete all the cache entries.
2. At the end of a "drush tome:static" run, \Drupal\tome_static\StaticGenerator::cleanupStaticDirectory will be called, which deletes any file that is expired in the cache. A file is considered expired if its cache has been invalidated. The reason this exists is that without this code there would be no way to know what exports need deleted. Deleting a node, for example, should result in the node's static export being deleted on the next run.

Upon reviewing Tome Static's codebase, I realized that there is probably a problem in when the cleanupStaticDirectory method is called. Right now it's called at the end of a build, which doesn't make any sense. Take this scenario for instance:

1. I run "drush tome:static" with fresh cache.
2. I invalidate the cache for something common, like a menu link. Almost every page's cache is invalidated.
3. I run "drush tome:static" again - the build goes OK, but at the end cleanupStaticDirectory is called and a lot of the export is deleted.

This may be a bit of a wild goose chase, but here's a patch that moves the "cleanupStaticDirectory" method to be called before the "prepareStaticDirectory" method. It looks like I already made this change to tome_static_cron_cron(), surprisingly. Let me know if it helps at all.

Edit: See comment below for a better perspective.

samuel.mortenson’s picture

Actually I've changed my mind after thinking of this for a bit - the current behavior is expected and I don't think a change is needed. To re-do my scenario from #2 with what's really going on:

1. I run "drush tome:static" with fresh cache
2. The page /home is generated
3. I invalidate the cache for something common, like a menu link. /home is invalidated
4. I run "drush tome:static" again
5. /home is re-generated since it isn't cached but still exists in the list of static paths
6. cleanupStaticDirectory is finally called to remove any path that's left over, which would only be invalidated paths that do not exist in the list of static paths. /home isn't deleted.

So going back to your request, I'm not sure exactly what's happening, but I would look into what paths are deleted in subsequent runs and determine how they got added to the static build (i.e. were they in the initial run, or were they found via crawling).

I have a theory about what could be happening, but since I don't have all the information about your site it's just a guess after reviewing all this code this morning. When Tome generates the original list of paths to process, entity paths are placeholdered for performance reasons, and are cached using their placeholdered path - which is something like _entity:node:en:1, not /node/1. You could wind up in a weird situation where, because of the crawling, /node/1 and _entity:node:en:1 are separately cached, and while _entity:node:en:1 is always re-generated /node/1 is left invalidated and is deleted. I'll think about this more before writing a patch.

Edit: Here's the relevant code from StaticCache.php that _should_ have prevented this from happening: https://git.drupalcode.org/project/tome/blob/8.x-1.x/modules/tome_static...

jpontani’s picture

So the patch, at least initially, seems to have resolved our disappearing/reappearing files act. I haven't had a chance to fully step through and figure out which files/paths were being deleted/recreated. My hunch is a similar scenario from what you mentioned, due to crawling. But rather than placeholdered paths and hard paths, I think it's because of Drupal paths and crawling having two identical paths, but with slight variances in either capitalization or trailing slash.

One consistent issue that happened early on before any changes, we had a taxonomy term with a URL alias (/finance) that was being generated but then deleted. In my troubleshooting, I noticed there were two cache entries for the same destination file, but the cache IDs were different (one was /Finance/, one was /finance). Hence the changes I made to how cache IDs were created. Both will create separate cache entries, because their URIs are different, but really they're the same thing.

protected function getCacheId($base_url, $original_path) {
  $cid_parts = [
    $base_url,
    rtrim($original_path, '/'),
  ];
  return strtolower(implode(':', $cid_parts));
}

That resolved the issue for the errors I was initially seeing, but there were more deletions that I couldn't pinpoint where it was occurring. The patch you provided basically ignores the duplicate destination issue above because it's deleting the files at the beginning of the run, and Tome just recreates them anyway.

samuel.mortenson’s picture

Status: Active » Needs review
StatusFileSize
new2.33 KB
new479 bytes

Thanks @jpontani, I'm glad my patch is helping, even if it wasn't a change I necessarily wanted to make in the first place.

Here's a new patch with your changes from #4 - let me know if it continues to work for you and I'll try to write tests and get it in. Even if I don't totally understand the problem, moving the code around like this seems OK.

rzaleski’s picture

We have found another potential pathway for disappearing files and we wanted to confirm this is plausible based on your understanding of the code base. Currently we have a path-pattern filter to remove "_entity:node" paths in specific:

--path-pattern='/^((\/(?!es\/|es$|sites\/default\/files|media\/|user\/|feed|admin\/|http|contextual\/|.*autocomplete$))).*$/'

Our original assumption was the _entity:node entries were duplicates of actual aliases (e.g. /somepath/somepage) and that by crawling "/" it would find all relevant pages.

It is important to know that our website structure is hierarchical (e.g. Page A -> Page B -> Page C).

So what was happening was Page C was modified. Tome would run and look at "/" (Page A), find Page B (which was not modified) and stop that "pathway". Therefore we'd end up with Page C never being updated (crawling would not find it and _entity:node was filtered out).

If we add back in _entity:node those pages are automatically re-processed (as we aren't reliant on the crawling of pages per se).

Does this seem likely?

samuel.mortenson’s picture

@rzaleski Yes, that seems like a possible scenario.

  • samuel.mortenson authored 7d63db4 on 8.x-1.x
    Issue #3103937 by samuel.mortenson, jpontani, rzaleski: Tome Static...
samuel.mortenson’s picture

Status: Needs review » Fixed

I know this patch fixes some issues, so I'm finally committing it. Please open new issues for any co-related problems. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.