Sent a summary of this discussion to the swh-devel list for input:

In T3627#72544, @douardda wrote:

B3 I am not convinced a "synthetic" flag on the Snapshot branch makes sense, or at least I find this name confusing, especially considering we already have a synthetic flag on Revision: it's not synthetic in the sense of it's not object crafted by SWH, it comes from the origin.

I think the main challenge here will be doing this in such a way that we don't have to do a fresh clone of swh-environment (and all associated repos) every time we build.

I would like us to conclude this discussion soon.

The permissions were missing for consumer groups, so no consumer could get started at all.

Looks sensible to me, thanks.

I've run alter system commands to bump these configuration variables in $DATADIR/postgresql.auto.conf, then ran a pg_reload_config():

The log is flooded with

2021-10-14 15:24:54.422 UTC [3951720] LOG:  checkpoints are occurring too frequently (28 seconds apart)
2021-10-14 15:24:54.422 UTC [3951720] HINT:  Consider increasing the configuration parameter "max_wal_size".

Ah, now that I read through this again; would it make sense for the zookeeper server to be called using the CNAME instead of the host FQDN ?

Looks good, except for a missing new TLS certificate, I think.

In T3658#72284, @olasd wrote:

We could argue that adding a separate, "virtual" lister instance for these bulk archived origins would make sense, but I don't know if it's worth the bother.

I was thinking of something ad-hoc such as:

In SWHIDv2, instead of having a hardcoded "pointer to another revision" directory entry type, we could enable pointers to more generic "unresolved external entities". When possible, we should make these pointers compatible with the current ExtID table, so that users of the data can look the contents of the pointed objects up lazily.

@vlorentz mentioned this idea in the context of T3655 (git loader global deduplication).

In T3635#72206, @douardda wrote:

Then I don't really get how this can help if we don't load revisions in topological order.

(I've removed T3653 as parent as this is a somewhat longer term endeavour. Not the topological sorting itself, but making sure that (most) existing revisions aren't dangling, before we can use this topological guarantee)

In D6458#167831, @anlambert wrote:

In D6458#167772, @olasd wrote:

In D6458#167771, @olasd wrote:

Yeah, sure, I don't have a problem with that.

(That is, I don't have a problem with these changes landing first, as long as we make sure that eventually we have a proper way of reproducing test failures that have come out of "random" fixtures.)

I might have a proper solution to reproduce test failures involving random fixtures. I am going to land that pile of diffs and submit a new one for reproducibility afterwards.

In D6458#167771, @olasd wrote:

Yeah, sure, I don't have a problem with that.

In D6458#167737, @vlorentz wrote:

@olasd Could you open a task, so anlambert can land this stack of diffs now before we discuss the next step?

Thanks for working on reducing the number of hypothesis fixtures!

Hmm, do we really want this to be open to the world with no authentication whatsoever? (which is what D6448 seems to be doing)

rebase

Fix revision -> release typo in release_add flush call

In T3625#71799, @olasd wrote:

While we're at it, we should probably be adding some thresholds in the buffer proxy for:

• cumulated length of messages for revisions and releases

I'll split off the new buffer thresholds in a new diff. This diff now only contains the (small) improvements to the buffer/filter proxies

Thanks!

Ah, another question I've been thinking about: should we go back to existing visits of git repositories and give them a new, pruned snapshot? Our data model now allows it: we can just append a new final OriginVisitStatus pointing at a pruned snapshot.

In D6424#166886, @ardumont wrote:

@dachary It'd be nice if you could describe what this is about in the commit message and
the diff description (if you actually provide a commit description, then when you create
the diff, the commit message is used as a description bootstrap). I know it's more work
for you but it happens that:

1. it helps the reviewers to have some context directly here (without having to follow

between a multitude of tasks. FYI, I've followed through the task but it's not enough,
i need to also dig in that arborescence of tasks).

2. is also how we are doing that in every other modules ;)

3. the curious could learn a thing or 2 even if they don't do a proper review.

Please and thanks in advance.

Cheers,

In T3627#71809, @zack wrote:

Thanks for your feedback @olasd. I see three main arguments raised there: (1) the raciness of archiving those data via other means (= related forks), (2) the completeness of our canvassing of synthetic refs, (3) annotating rather than not archiving "synthetic" refs.

For (1), sure, it's racy, hence we could lose stuff that gets removed from GitHub before we have the time to archive it. But this is a drop in the ocean in comparison with our lag/backlog.

rSPSITE6a233452cd48 fixed the prometheus node exporter.

Awesome, thanks for confirming this!

I'm asking this because using predictable origin-centric URLs is generally much more user friendly than having to use multiple APIs to look up the SWHID of a given object before being able to construct the URL, and one would have to always to dynamic API calls to generate the URL for browsing the "latest archival" of a given origin.

Just to be clear, you're looking to keep these URL working, but turn them into redirects over to swhid-centric URLs with context parameters (and drop the original view code from these URLs), correct?

While we're at it, we should probably be adding some thresholds in the buffer proxy for:

• cumulated length of messages for revisions and releases
• cumulated number of parents for revisions

(this also matches the fact that we've seen, on our main ingestion database, directory_add operations that would take multiple hours, and have knock-on effects on backups and replications because of the long-running insertion transactions)

So, after doing some more analysis of memory usage patterns on these edge case repositories, my suspicion is that the high memory usage is generally being caused by the loader processing batches of large directories, closely packed together, at the same time.

This should stay pending until we resolve the archiving policy discussion in T3627, so I'm marking it as such.

In T3627#71790, @rdicosmo wrote:

Yes, we must filter this stuff out (we discussed this issue with @zack some time ago)

In D6405#166673, @ardumont wrote:

This looks like an okay thing to do, but instead of only ignoring results (which would only cut down a third of the messages), we should probably be deactivating events completely on these workers.

Yes, I started with that config because i did not initially found the way to configure the send_events to False (or something).

Looks fine (i.e. the identifiers DeprecationWarnings are gone in tox, except for one that gets triggered by some pytest internal assertion rewrite).

Rather than doing this, we should probably disable worker task events altogether (that is, run celery worker without the --events/--task-events flag)

This looks like an okay thing to do, but instead of only ignoring results (which would only cut down a third of the messages), we should probably be deactivating events completely on these workers.