Two changes:
1. Don't write batch/roaring adds or removes when N is 0, because
a write of no bits is not a meaningful write.
2. When unmarshalling roaring things, if a roaring bitmap didn't
change many bits, treat it as having changed at least 1 bit per 8 bytes,
so an 8KB hunk of roaring data counts as 1K changes, which will
nudge us towards snapshotting. This should keep us from having
Large Files show up so much.
This was particularly noticeable on the existence field, which
tends to a steady state of "completely full" very quickly in a lot
of cases.
This PR adds a translationSyncer interface; I tried to include
comments in the code explaining what's going on. This is taken
from those comments:
translationSyncer provides an interface allowing a function
to notify the server that an action has occurred which requires
the translation sync process to be reset. In general, this
includes anything which modifies schema (add/remove index, etc),
or anything that changes the cluster topology (add/remove node).
I originally considered leveraging the broadcaster since that was
already in place and provides similar event messages, but the
broadcaster is really meant for notifiying other nodes, while
this is more akin to an internal message bus. In fact, I think
a future iteration on this may be to make it more generic so
it can act as an internal message bus where one of the messages
being published is "translationSyncReset".
This PR forces the non-coordinator nodes to reset their translation
sync (and therefore their own cosideration of read-only partitions)
any time they receive a `ClusterStatus` message. So basically, as the
cluster grows during the startup process, each node will reset their
translation sync.
This is NOT a good solution log term, but it should address the
immediate problem.
Things to note:
- the coordinator sync isn't getting reset, but that's ok, because the
immediate problem is a partition marked as read-only when it shouldn't
be; i.e. it's ok to have the inverse (a partition not marked as
read-only when it should be) because that partition won't receive
requests anyway.
- the last node to start is already correct and doesn't really need to
reset its sync.
- there are many other scenarios not covered by this fix.
Based on this theory:
```
i have another theory that i’m going to try to test.
this one would only apply in the case where a multi-node cluster is restarted with an existing, keyed index.
- start node0: it thinks it’s responsible for all partitions (nothing is read-only)
- start node1: it thinks it’s responsible for ~1/2 of the partitions and marks the other 1/2 as read-only
- start node2: it thinks it’s responsible for ~1/3 of the partitions and marks the other 2/3 as read-only
now if node0 is the coordinator receiving all translation requests, that still might not explain what’s happening, because in that case it would just do all the translating. i think. but either way, i should make sure that scenario is not happening, but i think it may be.
actually, that might explain it, because what would happen when the coordinator received a translation request, is that it would handle the 1/3 that it owned (now that the cluster is 3 nodes), and it would send the other 2/3 out to the other 2 nodes. but where it sent the requests wouldn’t line up with what the nodes thought they were responsible for based on the restart order
in this example, node 1 would receive requests for the wrong partitions
```
In the old unmarshal code, the decision to mark a thing as mapped (always
yes) happens separately from setting the mapping. What if this could ever
somehow possibly go wrong? Let's sanity-check that to be extra careful.
Log an error in the probably-irrelevant case where we ended up with
a file, but Stat failed, which shouldn't ever happen we hope anyway.
Also explicitly discard the status from RemapRoaringStorage in a case
where we don't care.
This is sort of prototype-ish, but the idea is that we use SetMaxMapCount
from syswrap, which already exists, to let us test edge cases like
"what happens if you only sometimes have mapped data".
We might have a problem with a stale mmap, and to try to narrow it down
a bit, we add some sanity-checking features and panic recovery to the
generation Transaction code.
This is pretty experimental.
In some cases, after a snapshot, if mmap fails, we could write
a duplicate of the bitmap to the file, creating cryptic "unknown
op type: 60" messages. This doesn't fix those files, but it stops
making them.
this avoids compounding floating point errors while summing up the
numbers, and means less logic needs to change. Should probably convert
min and max to use this approach as well, though they don't suffer
from the compounding error issue, it is simpler.
this involved adding an optional float value to the ValCount struct
which complicated result types, necessitated grpc changes, and needed
quite a few tests at different layers.
