Hacker News Reader

Comment by ccooffee | Hacker News Reader

> During a routine update to the critical elements snapshot data, an incomplete snapshot was inadvertently shared which removed several sites from the topology map.

I wish this went into more detail about how an incomplete snapshot was created and how the incomplete snapshot was valid-enough to sort-of work.

I'm supposing that whatever interchange format was in use does not have any "END" delimiters (e.g. closing quotes/braces), nor any checksumming to ensure the entire message was delivered. I'm mildly surprised that there wasn't a failsafe to prevent automatically replacing a currently-in-use snapshot with one that lacks many of the services. (Adding a "type 'yes, I mean this'" user interaction widget is my preferred approach to avoid this class of problem in admin interfaces.)

View on HN

One of the first things I did at Cloudflare was change the format of a file that contained vital information about the mapping between IPs and zones (no longer used these days) so that it had something like:

    START <count> <sha1>
    .
    .
    . 
    END

because it was just slurped up line by line assuming EOF was good.

Every mission critical file eventually gets version numbers and checksums.

And eventually a blockchain

Files can be immutable, cryptographically-verifiable, and distributed across servers without a blockchain.

Adding a blockchain to this would slow things down, add surface area for errors, and provide absolutely no value. This is true of every usage of blockchain other than creating tokens.

one reason why JSON is superior to things like TOML or YAML for these use-cases...

Not to worry, JSONL[1] fixes that ;)

In all seriousness - just dropping 1gb json file with N million records in one end probably isn't great either. I suppose one could somehow marry js and subresource integrity protection[2] to get a json serialized structure with hash integrity check. It would probably be a terrible idea.

[1] https://jsonlines.org/

[2] https://developer.mozilla.org/en-US/docs/Web/Security/Subres...

In my experience, this has been called "ndjson", or newline-delimited JSON. It's a remarkably effective and useful pattern.

Apparently jsonlines is only a website, whereas ndjson is a website+spec. Given the conceptual simplicity, this claim seems a bit dubious. Having two of these identical things, each one clawing for mindshare is dumb and counter-productive. Why can't we all be friendly and get along for the greater good? Oh well, it's par for the course.

http://ndjson.org/

That is probably the canonical term/format - I came across the reference to JSONL somewhere related to (de)serializing data for ai like:

https://platform.openai.com/docs/guides/fine-tuning/prepare-...

And googled/ducked from there.

Is it really better than just using an array, though? Now you essentially first parse the data/newline files, splitting blindly on "\n" - hoping that a) you've got the data, and b) that each record can be parsed as json - by an entirely different parser.

Meanwhile - a valid json file (Array of records) - can be quite quickly parsed by an optimized json parser, will error on wrong or truncated input, will (have to) handle reasonable whitespace?

> Is it really better than just using an array, though?

In many cases, absolutely yes it's better. Otherwise you can end up being forced to cram several GB or even 10's of GB of JSON into working memory. Not ideal.

Usually each ndjson file is generated by a single process, so there isn't much concern about invalid formatting making it's way in. It's one of those rare win-wins.

I could see how a language might have a streaming json parser, but where a naïve "for each line in file" might read the file as a string - allocating all that memory anyway?

Used properly, "For each line in file" constructs in the major languages (Python, Go, Java, C#, Javascript, and so on) are well tested and really do read only one single line at a time. Just be sure to avoid the usual pitfalls, e.g. don't turn all the lines into a list before starting iteration.

I actually recently tried out a python based steaming JSON parser I found on SO and it was actually even more inefficient (for both CPU and memory) than the built-in JSON package (probably because Python and not a C-module). It was also quite a complex state machine, tricky to catch all corner cases. The SO module had some bugs that I fixed, but I ended up abandoning the entire approach because it wasn't saving any memory.

NDJSON certainly is a much simpler approach, which I like and appreciate.

> Text editing programs call the first line of a text file "line 1". The first value in a JSON Lines file should also be called "value 1".

I wonder why not zero?

It's more likely that the snapshot was incomplete in that it was based on incomplete precursors, rather than that the message itself was truncated. The details of something like that aren't always appropriate for this kind of report (i.e. they usually create more questions than they answer for a reader unfamiliar with the basics of the system).

Google internally avoids use of ascii files, so the type of error you are suggesting is unlikley.

I suspect it was more a case of incorrect error handling in a loop... Eg.

    output = []
    try:
      for site in sites:
        output += process(site)
    except:
      print("error!")
    write_to_file(output)

Or it could have just been a recordio file that was being written by something that crashed in the middle of doing it, and the committed offset was at a valid end of record.

Really there's 1000 ways for this to happen all of which are going to sound obvious in retrospect but are easy to commit.

Even straight up Proto is vulnerable to this (either with an unlucky crash right between fields being actually written to a file, or when attempting to stream top-level fields).

End markers will help detect truncated files but not other kinds of brokenness, need checksums for those.

Also, I think the config files for certain off the shelf network devices don't come with end markers or checksums, and might not be very good at handling corrupted configs. The usual practice is to push updates to a small fraction of those, then wait and see what happens before proceeding further.

Unfortunately even checksums won't help you if you mistakenly dredge up old but valid config. That doesn't mean they're not worth doing, but we don't know, due to a lack of detail, if Google already has checksums or if they would even have helped in this situation.

I ran into a problem like this with a service that used YAML for their config file. Basically when I edited it and saved the service would automatically pick up the change and load the config. However, the save hadn't completed so it only read a partial file which was still valid because, YAML.

I still like XML and XSD. People look at me these days like I'm insane. But in this case a partially loaded XML document would not parse let alone pass schema validation.

Again vindicated. YAML needs to go away. It is misery. I'd rather have XSLT than Helm templates.

Both you and I are very aware that the XSLT + XML config file would have a tool for translating/generating it from a YAML; and most users would use that tool instead of configuring in XML.

Not if I amble around the office with a chair leg making menacing grunts they won't.

It's crazy how we still don't have the ability to atomically create/replace a file on the popular operating systems and filesystems , despite that being a super common use-case (without workarounds like renaming which have annoying limitations).

What would you want from an atomic replace of the contents of a file that rename() doesn't provide? Surely you've got to write the new contents somewhere before calling your atomic replace, and that's exactly what happens with rename/mv.

Rename does it and that it the workaround for issues like this.

How do you atomically replace an existing file using rename? I believe rename fails if the target file already exists, instead of atomically replacing it. Plus you end up with orphaned temporary files if you crash at the wrong point.

There is another workaround using unnamed tempfiles on Linux, but I think even that approach doesn't work for replacing files.

And then there is the issue that this creates a new file with the same name, instead of replacing the contents of the existing file. I think that's often not the desired behaviour regarding symlinks or inode ids.

And finally all those workarounds are overly complicated, so almost no application uses them, while pretty much every application that saves some kind of document (image, textfile, etc.) would benefit from atomic saving.

mv uses rename(). No issue with the target file existing beforehand. No issue with symlinks going stale, because they link to the name not the inode. Hard links will preserve the old file and still refer to that though (which is sometimes what you want, sometimes not). But for most, symlinks are far more common.

  $ echo "File 1" > file1
  $ echo "File 2" > file2
  $ ln -s file2 file3
  $ cat file3
  File 2
  $ echo "File 4" > file4
  $ mv file4 file2
  $ cat file3
  File 4

>I'm supposing that whatever interchange format was in use does not have any "END" delimiters (e.g. closing quotes/braces), nor any checksumming to ensure the entire message was delivered.

Those only ensure you get the whole message, not that the message makes sense.

Quite true. I was assuming that the incomplete snapshot was a transmission error or storage error. It's quite possible that the bug was an error of omission inside the data itself (e.g. someone accidentally removed an important key-value mapping from the data generator).

You're assuming it just stopped abruptly instead of just was missing data.

No. That's not how it would be done at google. they invented a binary protocol to handle things like this reliably. And that protocol goes over an integrity-checking network transport. It's more likely an odd edge case occurred and somehow got past the normal QC checks- say, an RPC returned an error but the error handler didn't do a retry, it just fell through.

Reply on news.ycombinator.com