Found while working #6340 (PR #6347), which added the first TimeSeries coverage to CHECK DATABASE. This is
a separate defect in the schema load path, not in the checker, which is why it was not fixed there.
What happens
Corrupt one byte of a .ts.sealed file's header magic, close the database, reopen it. The database opens
cleanly and the TimeSeries type is gone:
totalTimeSeriesTypes = 0
database.getSchema().existsType("Cpu") = false
CHECK DATABASE warnings = []
No exception reaches the caller, no warning, no log line an operator would look for. A type that holds every
sample the application wrote simply is not there any more, and the next write creates a fresh empty one.
Why
TimeSeriesSealedStore.loadDirectory() throws IOException("Invalid sealed store magic: ...") on a bad
magic. That propagates through the TimeSeriesShard constructor to TimeSeriesEngine, so
LocalTimeSeriesType.initEngine() throws, and LocalSchema.readConfiguration() wraps it
(LocalSchema.java:1874):
case "t" -> {
final LocalTimeSeriesType tsType = new LocalTimeSeriesType(this, typeName);
tsType.fromJSON(schemaType);
try {
tsType.initEngine();
} catch (final IOException e) {
throw new ConfigurationException("Error initializing TimeSeries engine for type '" + typeName + "'", e);
}
...
yield tsType;
}this.types.put(typeName, type) is only reached after the yield, so the throw leaves the type out of the
map. Something upstream then swallows the ConfigurationException and lets the open complete - that swallow
is the part worth tracing, because on its own the throw would at least have failed the open loudly.
Why CHECK DATABASE cannot cover this
DatabaseChecker walks database.getSchema().getTypes(). A type that is not in the schema is not walkable
by anything, so the new TimeSeries pass in #6347 reports nothing here and correctly so - the gap has to be
closed where the type is dropped, not where it would have been checked. #6347 leaves a comment at the test
that would have covered it rather than pinning the current behaviour, since a test asserting "the type
vanishes" would enshrine it.
The design question
Three defensible answers, and this needs a decision before code:
- Refuse the open. Loudest and safest against data loss - nobody writes into a fresh empty type by
accident - but one damaged derived file takes the whole database offline, and a sealed store IS derived:
each HA node rebuilds its own by compaction, so the data is not necessarily lost at all.
- Register the type with its engine unavailable, so it appears in the schema,
CHECK DATABASE can report
it, and reads/writes against it fail with a message naming the file. Keeps the blast radius at one type.
- Quarantine the sealed file and open with the mutable half only, which is what the format could actually
support - the mutable .tstb pages are replicated and intact, so the type would come back with its
uncompacted data and the sealed blocks would be rebuilt.
My preference is (2) as the immediate fix, because it is the one that makes the state visible without
deciding what to do about it, and it is what lets the #6347 check report the type at all. (3) is the better
end state but wants its own discussion about when a node may discard derived data.
Whatever is chosen, the swallowed ConfigurationException should be tracked down independently: a schema
load that fails to construct a type and then reports success is a bad shape regardless of which type it is.
Reproduction
Append rows to a TimeSeries type, compactAll() so a sealed file exists, close, flip byte 0 of
<type>_shard_0.ts.sealed, reopen. See the removed-test comment in
Issue6340TimeSeriesCheckDatabaseTest for the exact shape.
Found while working #6340 (PR #6347), which added the first TimeSeries coverage to
CHECK DATABASE. This isa separate defect in the schema load path, not in the checker, which is why it was not fixed there.
What happens
Corrupt one byte of a
.ts.sealedfile's header magic, close the database, reopen it. The database openscleanly and the TimeSeries type is gone:
No exception reaches the caller, no warning, no log line an operator would look for. A type that holds every
sample the application wrote simply is not there any more, and the next write creates a fresh empty one.
Why
TimeSeriesSealedStore.loadDirectory()throwsIOException("Invalid sealed store magic: ...")on a badmagic. That propagates through the
TimeSeriesShardconstructor toTimeSeriesEngine, soLocalTimeSeriesType.initEngine()throws, andLocalSchema.readConfiguration()wraps it(
LocalSchema.java:1874):this.types.put(typeName, type)is only reached after theyield, so the throw leaves the type out of themap. Something upstream then swallows the
ConfigurationExceptionand lets the open complete - that swallowis the part worth tracing, because on its own the throw would at least have failed the open loudly.
Why
CHECK DATABASEcannot cover thisDatabaseCheckerwalksdatabase.getSchema().getTypes(). A type that is not in the schema is not walkableby anything, so the new TimeSeries pass in #6347 reports nothing here and correctly so - the gap has to be
closed where the type is dropped, not where it would have been checked. #6347 leaves a comment at the test
that would have covered it rather than pinning the current behaviour, since a test asserting "the type
vanishes" would enshrine it.
The design question
Three defensible answers, and this needs a decision before code:
accident - but one damaged derived file takes the whole database offline, and a sealed store IS derived:
each HA node rebuilds its own by compaction, so the data is not necessarily lost at all.
CHECK DATABASEcan reportit, and reads/writes against it fail with a message naming the file. Keeps the blast radius at one type.
support - the mutable
.tstbpages are replicated and intact, so the type would come back with itsuncompacted data and the sealed blocks would be rebuilt.
My preference is (2) as the immediate fix, because it is the one that makes the state visible without
deciding what to do about it, and it is what lets the #6347 check report the type at all. (3) is the better
end state but wants its own discussion about when a node may discard derived data.
Whatever is chosen, the swallowed
ConfigurationExceptionshould be tracked down independently: a schemaload that fails to construct a type and then reports success is a bad shape regardless of which type it is.
Reproduction
Append rows to a TimeSeries type,
compactAll()so a sealed file exists, close, flip byte 0 of<type>_shard_0.ts.sealed, reopen. See the removed-test comment inIssue6340TimeSeriesCheckDatabaseTestfor the exact shape.