We encountered a situation in simulation where the disk queue was in the following state
+------------+------------+
| page 1 | page 2 |
+------------+------------+
|rec |.......|rec |.......|
+------------+------------+
0..85 4096..4181
^. ^__ ^
popped. committed pushed
and we attempted to pop up to 4096, i.e. everything before page 2. This triggered
one of the assertions in the disk queue code which was meant to catch tlog logic
bugs where we pop too much.
The issue, though, is the accounting of the commit location in the disk queue.
While we only pushed records through position 85, we committed the entire page.
Attempts to pop everything before page 2 should have succeeded since we're not
attempting to pop any uncommitted data.
The solution is to fix the commit location accounting in the disk queue to round
up to the next page, to reflect the reality that we only commit entire pages.
This bug was discovered in the first place by introducing a delay into the commit
queue loop during simulation testing. That delay is included in this change.
We also noticed that getNextCommitLocation() was incorrect. Since there are no
users of that function, we've removed it entirely.
It was assumed that the payloadSize field in the page header was 32 bits.
This was checked by a static_assert on the size of the PageHeader struct.
This change makes that field size explicit, rather than assuming that
sizeof(int) == sizeof(int32_t).
Both push() and pushAndCommit() now require reference-counted pageData and file->write() is wrapped in uncancellable(holdWhile(pageData, ..)). To support this with no additional overhead, StringBuffer returns a Standalone reference to its contents.
* proof of concept
* use code-probe instead of test
* code probe working on gcc
* code probe implemented
* renamed TestProbe to CodeProbe
* fixed refactoring typo
* support filtered output
* print probes at end of simulation
* fix missed probes print
* fix deduplication
* Fix refactoring issues
* revert bad refactor
* make sure file paths are relative
* fix more wrong refactor changes
We had been disabling -Wdelete-non-virtual-dtor, because this seems to be done intentionally in the generated code of the actor compiler. I spent some time trying to rewrite it in a way that doesn't literally delete/destroy through a pointer to a base class without a virtual destructor, but I was unable to come up with something that passes correctness. My best guess is that we do this so that we can destroy actor state classes, call callbacks registered on the actor SAV, and then destroy the SAV.
Anyway now we'll detect new usages of deleting through a pointer to a base class without a virtual destructor.