Atomic version map update

[joe-iddon]

Reference Issues/PRs

What does this implement or fix?

Writing the symbol ref key should always be the last step for all public methods in version_map.hpp to avoid race conditions where the symbol ref key points temporarily to, say, a TOMBSTONE_ALL version key in the process of a call to write_and_prune_previous. This means that readers temporarily get invalid data (or no data!).

In particular, if reading a library with incompletes on, then we can get a Stream descriptor not found in pipeline context error due to this race condition (need incompletes to keep reading even if no versions exist whereas for a symbol with incompletes off we would get a more informative error, and also need columns specified otherwise we get a very closely related Normalization metdata not found exception for the same reason but errors at a different point in the code).

Warning

API Changes

• Rather than the obtuse Stream descriptor not found/Normalization metadata not found errors, we now give a more sensible error message: E_NO_SYMBOL_DATA: read_dataframe_impl: read returned no data for symbol {} which is a more informative failure mode in the case of reading an incomplete=True symbol, but there being no data available.
• Changed logging from INFO to DEBUG when compacting a symbol (top-level API giving INFO log message does not make senese):

- log::version().info("Compacting incomplete symbol {}", stream_id);
+ log::version().debug("Compacting incomplete symbol {}", stream_id);

Example race condition

Previously (one possible race condition)

• the new version is created and APPEND_DATA keys deleted
• a TOMBSTONE_ALL version was added to the version list
• the version ref key was updated to point at that new version
• the new version was added to the version list
• the version ref key updated

Now:

• the new version is created and APPEND_DATA keys deleted
• a TOMBSTONE_ALL version was added to the version list
• the new version was added to the version list
• the version ref key updated

Implementation

Main changes are to version_map.hpp: Before, whenever it made sense to write the sym ref key, it was done. Following these two rules, we guarantee that all updates are now atomic:

• Public methods write the symbol ref key at most once.
• Private methods don't write the symbol ref key.
• All methods only call private methods.

There are two exceptions to these rules:

• Any method can call a public method if it clearly doesn't write the symbol ref key.
• Public methods can wrap other public methods if they don't write the symbol ref key again themselves.
  Examples of exceptions:
• delete_all_versions is public, but can call the public tomstone_from_key_or_all since all it does is convert the return type
  fix_ref_key is public but calls scan_and_rewrite which is also public. This is fine as it just does a read of the ref key first, but then scan_and_rewrite does the rest of the work.

Any other comments?

• We should really delete the APPEND_DATA keys as the final step in the process.

Checklist

Checklist for code changes...

• Have you updated the relevant docstrings, documentation and copyright notice?
• Is this contribution tested against all ArcticDB's features?
• Do all exceptions introduced raise appropriate error messages?
• Are API changes highlighted in the PR description?
• Is the PR labelled as enhancement or bug so it appears in autogenerated release notes?