Service node list store historical states #754
Conversation
Doy-lee
force-pushed
the
ServiceNodeListStoreHistoricalStates
branch
from
608dc46
to
aea0b1d
Compare
|
|
||
| for (const auto& event : data_in.events) | ||
| for (size_t i = start_index, last_index = new_data_in.states.size() - 1; | ||
| i < new_data_in.states.size(); |
There was a problem hiding this comment.
Since you've got it here already, this could just be: i <= last_index
| return false; | ||
| } | ||
| state_serialized &source = new_data_in.states[i]; | ||
| state_t *dest = (i == last_index) ? &m_state : &m_state_history[i - start_index]; |
There was a problem hiding this comment.
This is minor, but you can use a reference here instead of a pointer and avoid the pointer dereferences below:
state_t &dest = (i == last_index) ? m_state : m_state_history[i - start_index];
dest.height = source.height;
// etc.
src/blockchain_db/lmdb/db_lmdb.cpp
Outdated
| size_t constexpr NUM_CHECKPOINTS = service_nodes::CHECKPOINT_NUM_CHECKPOINTS_FOR_CHAIN_FINALITY; | ||
| std::vector<checkpoint_t> const checkpoints = get_checkpoints_range(height(), 0, NUM_CHECKPOINTS); | ||
| if (checkpoints.size() == NUM_CHECKPOINTS) | ||
| checkpoints.back().height; |
| m_transient_state.rollback_events.pop_front(); | ||
| } | ||
| m_transient_state.rollback_events.push_front(std::unique_ptr<rollback_event>(new prevent_rollback(cull_height))); | ||
| uint64_t const cull_height = std::max(checkpoint_immutable_height, conservative_height); |
There was a problem hiding this comment.
If this conservative_height ends up being the maximum then this will end up removing states that might be reorged, and such a reorg then becomes expensive because it triggers a full chain scan. Can we store some more pieces of history here -- perhaps something like storing every (H%100==0) state for the last 1000 heights (i.e. 10 values), and then every (H%10000==0) before that? That way even the worst case fallback is a rescan of a hundred blocks rather than ~300k blocks.
| bool long_term_storage = (it->height % 10000 == 0); | ||
| if (long_term_storage) continue; | ||
| it = m_state_history.erase(it); | ||
| it--; |
There was a problem hiding this comment.
This -- is not valid in some cases: if erase returns an iterator to the new head of the deque then the -- is going to put it in an invalid state (i.e. before the beginning).
Better approach:
while (it != m_state_history.end() && it->height <= start_height) {
if (it->height % 10000 == 0)
it++;
else
it = m_state_history.erase(it);
}There was a problem hiding this comment.
Can it return an iterator to begin? According to docs (https://en.cppreference.com/w/cpp/container/vector/erase), if you erase the last element you get the end iterator and end() - 1 gives you begin()?
Also the advantage to having a deque here isn't clear anymore, so was also gonna change it to a vector. Was running this in the office to see what the cost of this was on mainnet but was still syncing before I left so gonna wait for the results of that
Increase the range to mitigate frequent rescanning when reorganising, technically with checkpointing we only need up to the 2nd oldest checkpoint, but I'm sure there's some case we're forgetting that would benefit from a larger rollback range. Also considering there were only 160 events in the month with a mass deregistration- keeping 2 days worth of rollbacks around is not going to be too expensive. Start storing service node list state for past blocks Separate quorums and reduce storage to 1 days worth Don't use pointer, declare m_state by value Fix rebase to dev Fix double serialise of service node state and expensive blob copy std::move quorums into storage, reserve vector size Don't +1 checkpoint finality height Code review Code review 2
Avoid constant reallocation.
Doy-lee
force-pushed
the
ServiceNodeListStoreHistoricalStates
branch
from
cb3d617
to
7f9b583
Compare
@jagerman
Storing the max(30, latest 3 checkpoints) blocks worth of service node list.
Currently sitting at 8mb for 30 blocks.