https://github.com/facebook/rocksdb
Revision 230660be7333a2fc6069fd860f1701fb5dfc5243 authored by Peter Dillinger on 18 November 2021, 19:42:12 UTC, committed by Facebook GitHub Bot on 18 November 2021, 19:43:44 UTC
Summary: * Checksums are now checked on meta blocks unless specifically suppressed or not applicable (e.g. plain table). (Was other way around.) This means a number of cases that were not checking checksums now are, including direct read TableProperties in Version::GetTableProperties (fixed in meta_blocks ReadTableProperties), reading any block from PersistentCache (fixed in BlockFetcher), read TableProperties in SstFileDumper (ldb/sst_dump/BackupEngine) before table reader open, maybe more. * For that to work, I moved the global_seqno+TableProperties checksum logic to the shared table/ code, because that is used by many utilies such as SstFileDumper. * Also for that to work, we have to know when we're dealing with a block that has a checksum (trailer), so added that capability to Footer based on magic number, and from there BlockFetcher. * Knowledge of trailer presence has also fixed a problem where other table formats were reading blocks including bytes for a non-existant trailer--and awkwardly kind-of not using them, e.g. no shared code checking checksums. (BlockFetcher compression type was populated incorrectly.) Now we only read what is needed. * Minimized code duplication and differing/incompatible/awkward abstractions in meta_blocks.{cc,h} (e.g. SeekTo in metaindex block without parsing block handle) * Moved some meta block handling code from table_properties*.* * Moved some code specific to block-based table from shared table/ code to BlockBasedTable class. The checksum stuff means we can't completely separate it, but things that don't need to be in shared table/ code should not be. * Use unique_ptr rather than raw ptr in more places. (Note: you can std::move from unique_ptr to shared_ptr.) Without enhancements to GetPropertiesOfAllTablesTest (see below), net reduction of roughly 100 lines of code. Pull Request resolved: https://github.com/facebook/rocksdb/pull/9163 Test Plan: existing tests and * Enhanced DBTablePropertiesTest.GetPropertiesOfAllTablesTest to verify that checksums are now checked on direct read of table properties by TableCache (new test would fail before this change) * Also enhanced DBTablePropertiesTest.GetPropertiesOfAllTablesTest to test putting table properties under old meta name * Also generally enhanced that same test to actually test what it was supposed to be testing already, by kicking things out of table cache when we don't want them there. Reviewed By: ajkr, mrambacher Differential Revision: D32514757 Pulled By: pdillinger fbshipit-source-id: 507964b9311d186ae8d1131182290cbd97a99fa9
1 parent f429466
Tip revision: 230660be7333a2fc6069fd860f1701fb5dfc5243 authored by Peter Dillinger on 18 November 2021, 19:42:12 UTC
Improve / clean up meta block code & integrity (#9163)
Improve / clean up meta block code & integrity (#9163)
Tip revision: 230660b
File | Mode | Size |
---|---|---|
.circleci | ||
.github | ||
buckifier | ||
build_tools | ||
cache | ||
cmake | ||
coverage | ||
db | ||
db_stress_tool | ||
docs | ||
env | ||
examples | ||
file | ||
fuzz | ||
hdfs | ||
include | ||
java | ||
logging | ||
memory | ||
memtable | ||
microbench | ||
monitoring | ||
options | ||
plugin | ||
port | ||
table | ||
test_util | ||
third-party | ||
tools | ||
trace_replay | ||
util | ||
utilities | ||
.clang-format | -rw-r--r-- | 138 bytes |
.gitignore | -rw-r--r-- | 1.0 KB |
.lgtm.yml | -rw-r--r-- | 67 bytes |
.travis.yml | -rw-r--r-- | 9.5 KB |
.watchmanconfig | -rw-r--r-- | 130 bytes |
AUTHORS | -rw-r--r-- | 322 bytes |
CMakeLists.txt | -rw-r--r-- | 50.0 KB |
CODE_OF_CONDUCT.md | -rw-r--r-- | 3.3 KB |
CONTRIBUTING.md | -rw-r--r-- | 706 bytes |
COPYING | -rw-r--r-- | 17.7 KB |
DEFAULT_OPTIONS_HISTORY.md | -rw-r--r-- | 1.5 KB |
DUMP_FORMAT.md | -rw-r--r-- | 763 bytes |
HISTORY.md | -rw-r--r-- | 197.6 KB |
INSTALL.md | -rw-r--r-- | 7.8 KB |
LANGUAGE-BINDINGS.md | -rw-r--r-- | 1.2 KB |
LICENSE.Apache | -rw-r--r-- | 11.1 KB |
LICENSE.leveldb | -rw-r--r-- | 1.5 KB |
Makefile | -rw-r--r-- | 89.9 KB |
PLUGINS.md | -rw-r--r-- | 320 bytes |
README.md | -rw-r--r-- | 2.0 KB |
ROCKSDB_LITE.md | -rw-r--r-- | 1.0 KB |
TARGETS | -rw-r--r-- | 59.5 KB |
USERS.md | -rw-r--r-- | 8.1 KB |
Vagrantfile | -rw-r--r-- | 1017 bytes |
WINDOWS_PORT.md | -rw-r--r-- | 12.5 KB |
appveyor.yml | -rw-r--r-- | 3.3 KB |
defs.bzl | -rw-r--r-- | 1.9 KB |
issue_template.md | -rw-r--r-- | 294 bytes |
src.mk | -rw-r--r-- | 43.2 KB |
thirdparty.inc | -rw-r--r-- | 7.8 KB |
![swh spinner](/static/img/swh-spinner.gif)
Computing file changes ...