Skip to content

Commit

Permalink
[release/2.x] Cherry pick: Fix issues with snapshot generation for CH…
Browse files Browse the repository at this point in the history
…AMP (#4730) (#4747)
  • Loading branch information
jumaffre authored Dec 14, 2022
1 parent 5a428c8 commit cd19fe9
Show file tree
Hide file tree
Showing 9 changed files with 150 additions and 137 deletions.
5 changes: 4 additions & 1 deletion .daily_canary
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
It's a new dawn, there is a new heatwave......
___ ___
(- *) (O o) | Y ^ O
( V ) < V > O +---'---'
/--x-m- /--m-m---xXx--/--yy---
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
### Fixed

- Node-to-node channels no longer check certificate expiry times. This previously caused "Peer certificate verification failed" error messages when node or service certs expired. (#4733)
- Fixed issue where invalid snapshots could be generated depending on the pattern of additions/removals of keys in a given key-value map (#4730).

## [2.0.12]

Expand Down
1 change: 1 addition & 0 deletions include/ccf/ds/logger.h
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ namespace logger

static inline void default_init()
{
get_loggers().clear();
add_text_console_logger();
}
#endif
Expand Down
57 changes: 32 additions & 25 deletions src/ds/champ_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,10 @@ namespace champ
const auto& entry = bin[i];
if (k == entry->key)
{
const auto diff = map::get_serialized_size_with_padding(entry->key) +
map::get_serialized_size_with_padding(entry->value);
bin[i] = std::make_shared<Entry<K, V>>(k, v);
return map::get_size(k) + map::get_size(v);
return diff;
}
}
bin.push_back(std::make_shared<Entry<K, V>>(k, v));
Expand All @@ -143,8 +145,8 @@ namespace champ
const auto& entry = bin[i];
if (k == entry->key)
{
const auto diff =
map::get_size(entry->key) + map::get_size(entry->value);
const auto diff = map::get_serialized_size_with_padding(entry->key) +
map::get_serialized_size_with_padding(entry->value);
bin.erase(bin.begin() + i);
return diff;
}
Expand Down Expand Up @@ -211,6 +213,7 @@ namespace champ
return node_as<SubNodes<K, V, H>>(c_idx)->getp(depth + 1, hash, k);
}

// Returns serialised size of overwritten (k,v) if k exists, 0 otherwise
size_t put_mut(SmallIndex depth, Hash hash, const K& k, const V& v)
{
const auto idx = mask(hash, depth);
Expand Down Expand Up @@ -246,8 +249,8 @@ namespace champ
const auto& entry0 = node_as<Entry<K, V>>(c_idx);
if (k == entry0->key)
{
auto current_size = map::get_size_with_padding(entry0->key) +
map::get_size_with_padding(entry0->value);
auto current_size = map::get_serialized_size_with_padding(entry0->key) +
map::get_serialized_size_with_padding(entry0->value);
nodes[c_idx] = std::make_shared<Entry<K, V>>(k, v);
return current_size;
}
Expand Down Expand Up @@ -297,6 +300,7 @@ namespace champ
std::make_shared<SubNodes<K, V, H>>(std::move(node)), r);
}

// Returns serialised size of removed (k,v) if k exists, 0 otherwise
size_t remove_mut(SmallIndex depth, Hash hash, const K& k)
{
const auto idx = mask(hash, depth);
Expand All @@ -311,8 +315,8 @@ namespace champ
if (entry->key != k)
return 0;

const auto diff = map::get_size_with_padding(entry->key) +
map::get_size_with_padding(entry->value);
const auto diff = map::get_serialized_size_with_padding(entry->key) +
map::get_serialized_size_with_padding(entry->value);
nodes.erase(nodes.begin() + c_idx);
data_map = data_map.clear(idx);
return diff;
Expand Down Expand Up @@ -437,9 +441,10 @@ namespace champ
if (r.second == 0)
size_++;

const auto size_change =
(map::get_size_with_padding(key) + map::get_size_with_padding(value)) -
const auto size_change = (map::get_serialized_size_with_padding(key) +
map::get_serialized_size_with_padding(value)) -
r.second;

return Map(std::move(r.first), size_, size_change + serialized_size);
}

Expand Down Expand Up @@ -492,17 +497,14 @@ namespace champ
{
std::vector<KVTuple> ordered_state;
ordered_state.reserve(map.size());
size_t size = 0;
size_t serialized_size = 0;

map.foreach([&](auto& key, auto& value) {
map.foreach([&ordered_state, &serialized_size](auto& key, auto& value) {
K* k = &key;
V* v = &value;
uint32_t ks = map::get_size(key);
uint32_t vs = map::get_size(value);
uint32_t key_size = ks + map::get_padding(ks);
uint32_t value_size = vs + map::get_padding(vs);

size += (key_size + value_size);
uint32_t key_size = map::get_serialized_size_with_padding(key);
uint32_t value_size = map::get_serialized_size_with_padding(value);
serialized_size += (key_size + value_size);

ordered_state.emplace_back(k, static_cast<Hash>(H()(key)), v);

Expand All @@ -517,25 +519,30 @@ namespace champ
});

CCF_ASSERT_FMT(
size == map.get_serialized_size(),
"size:{}, map->size:{} ==> count:{}, vect:{}",
size,
serialized_size == map.get_serialized_size(),
"Serialized size:{}, map.get_serialized_size():{} (map count:{}, "
"ordered state count:{})",
serialized_size,
map.get_serialized_size(),
map.size(),
ordered_state.size());

for (const auto& p : ordered_state)
{
// Serialize the key
uint32_t key_size = map::serialize(*p.k, data, size);
map::add_padding(key_size, data, size);
uint32_t key_size = map::serialize(*p.k, data, serialized_size);
map::add_padding(key_size, data, serialized_size);

// Serialize the value
uint32_t value_size = map::serialize(*p.v, data, size);
map::add_padding(value_size, data, size);
uint32_t value_size = map::serialize(*p.v, data, serialized_size);
map::add_padding(value_size, data, serialized_size);
}

CCF_ASSERT_FMT(size == 0, "buffer not filled, remaining:{}", size);
CCF_ASSERT_FMT(
serialized_size == 0,
"Serialization buffer is not complete, remaining:{}/{}",
serialized_size,
map.get_serialized_size());
}
};

Expand Down
2 changes: 1 addition & 1 deletion src/ds/map_serializers.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ namespace map
}

template <class T>
static size_t get_size_with_padding(const T& t)
static size_t get_serialized_size_with_padding(const T& t)
{
const uint32_t t_size = get_size(t);
return t_size + get_padding(t_size);
Expand Down
4 changes: 2 additions & 2 deletions src/ds/rb_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ namespace rb
_rgt(rgt)
{
total_size = 1;
total_serialized_size =
map::get_size_with_padding(key) + map::get_size_with_padding(val);
total_serialized_size = map::get_serialized_size_with_padding(key) +
map::get_serialized_size_with_padding(val);
if (lft)
{
total_size += lft->size();
Expand Down
Loading

0 comments on commit cd19fe9

Please sign in to comment.