exchange.remove() returns true if a key has already been removed
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Akiban Persistit |
Fix Released
|
Medium
|
Nathan Williams |
Bug Description
In the server code, when we maintain GI row counts, we decrement the counter iff our attempt to remove the current key causes it to actually be removed. This happens in OperatorStoreGI
I originally tried this code:
if (exchange.remove()) {
}
That didn't work as expected, but this does:
if (exchange.
}
Looking at the persistit UI, the key is defined but has an unpruned MVV that marks it as deleted:
{"John"
(The second one of those antivalues is the one caused by the if (exchange.
To reproduce:
- server trunk at revno 1430, persistit at 3.0.1-SNAPSHOT.220
- turn persistit UI on
- put a breakpoint in NewGiUpdateIT.
- once that's hit, put a breakpoint at OperatorStoreGI
- you should see the antivalued key, and exchange.remove() should return true.
Fwiw, even with the fetch-then-remove version, exchange.remove() still returns true.
Changed in akiban-persistit: | |
status: | Confirmed → Fix Committed |
Changed in akiban-persistit: | |
status: | Fix Committed → Fix Released |
Changed in akiban-persistit: | |
milestone: | samba → 1.1 |
Changed in akiban-persistit: | |
milestone: | 1.1 → 3.0.3 |
Changed in akiban-server: | |
assignee: | nobody → Yuval Shavit (yshavit) |
Changed in akiban-server: | |
milestone: | 1.1 → 1.2.1 |
status: | Fix Committed → Fix Released |
information type: | Private → Public |
Adding server as also-affected, since our code (at least in OperatorStoreGI Handler) uses the workaround of not relying on exchange.remove()'s return value. As far as I can tell, no other usage in the server relies on this return value. So the server is functionally correct here, but the code is a bit more verbose than it should be. My guess is that the performance hit is negligible, since a correct implementation of exchange.remove() would have to look at the MVVs to decide its return value, same as the current fetch.