From d7736ac27af92a80f924372ce98e54db98ee741a Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 10 Mar 2020 21:27:46 +0000 Subject: [PATCH 01/12] . --- src/ds/dllist.h | 257 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 257 insertions(+) create mode 100644 src/ds/dllist.h diff --git a/src/ds/dllist.h b/src/ds/dllist.h new file mode 100644 index 000000000000..57126cf7f91f --- /dev/null +++ b/src/ds/dllist.h @@ -0,0 +1,257 @@ +#pragma once + +#include +#include +#include + +namespace snmalloc +{ + /** + * The type used for an address. Currently, all addresses are assumed to be + * provenance-carrying values and so it is possible to cast back from the + * result of arithmetic on an address_t. Eventually, this will want to be + * separated into two types, one for raw addresses and one for addresses that + * can be cast back to pointers. + */ + using address_t = uintptr_t; + + /** + * Invalid pointer class. This is similar to `std::nullptr_t`, but allows + * other values. + */ + template + struct InvalidPointer + { + /** + * Equality comparison. Two invalid pointer values with the same sentinel + * are always the same, invalid pointer values with different sentinels are + * always different. + */ + template + constexpr bool operator==(const InvalidPointer&) + { + return Sentinel == OtherSentinel; + } + /** + * Equality comparison. Two invalid pointer values with the same sentinel + * are always the same, invalid pointer values with different sentinels are + * always different. + */ + template + constexpr bool operator!=(const InvalidPointer&) + { + return Sentinel != OtherSentinel; + } + /** + * Implicit conversion, creates a pointer with the value of the sentinel. + * On CHERI and other provenance-tracking systems, this is a + * provenance-free integer and so will trap if dereferenced, on other + * systems the sentinel should be a value in unmapped memory. + */ + template + operator T*() const + { + return reinterpret_cast(Sentinel); + } + /** + * Implicit conversion to an address, returns the sentinel value. + */ + operator address_t() const + { + return Sentinel; + } + }; + + template + class DLList + { + private: + static_assert( + std::is_same::value, "T->prev must be a T*"); + static_assert( + std::is_same::value, "T->next must be a T*"); + + T* head = Terminator(); + T* tail = Terminator(); + + public: + virtual ~DLList() + { + clear(); + } + + DLList() = default; + + DLList(DLList&& o) noexcept + { + head = o.head; + tail = o.tail; + + o.head = nullptr; + o.tail = nullptr; + } + + DLList& operator=(DLList&& o) noexcept + { + head = o.head; + tail = o.tail; + + o.head = nullptr; + o.tail = nullptr; + return *this; + } + + bool is_empty() + { + return head == Terminator(); + } + + T* get_head() + { + return head; + } + + T* get_tail() + { + return tail; + } + + T* pop() + { + T* item = head; + + if (item != Terminator()) + remove(item); + + return item; + } + + T* pop_tail() + { + T* item = tail; + + if (item != Terminator()) + remove(item); + + return item; + } + + void insert(T* item) + { +#ifndef NDEBUG + debug_check_not_contains(item); +#endif + + item->next = head; + item->prev = Terminator(); + + if (head != Terminator()) + head->prev = item; + else + tail = item; + + head = item; +#ifndef NDEBUG + debug_check(); +#endif + } + + void insert_back(T* item) + { +#ifndef NDEBUG + debug_check_not_contains(item); +#endif + + item->prev = tail; + item->next = Terminator(); + + if (tail != Terminator()) + tail->next = item; + else + head = item; + + tail = item; +#ifndef NDEBUG + debug_check(); +#endif + } + + void remove(T* item) + { +#ifndef NDEBUG + debug_check_contains(item); +#endif + + if (item->next != Terminator()) + item->next->prev = item->prev; + else + tail = item->prev; + + if (item->prev != Terminator()) + item->prev->next = item->next; + else + head = item->next; + +#ifndef NDEBUG + debug_check(); +#endif + } + + void clear() + { + while (head != nullptr) + { + auto c = head; + remove(c); + delete c; + } + } + + void debug_check_contains(T* item) + { +#ifndef NDEBUG + debug_check(); + T* curr = head; + + while (curr != item) + { + assert(curr != Terminator()); + curr = curr->next; + } +#else + UNUSED(item); +#endif + } + + void debug_check_not_contains(T* item) + { +#ifndef NDEBUG + debug_check(); + T* curr = head; + + while (curr != Terminator()) + { + assert(curr != item); + curr = curr->next; + } +#else + UNUSED(item); +#endif + } + + void debug_check() + { +#ifndef NDEBUG + T* item = head; + T* prev = Terminator(); + + while (item != Terminator()) + { + assert(item->prev == prev); + prev = item; + item = item->next; + } +#endif + } + }; +} // namespace snmalloc \ No newline at end of file From 8a43c02c4b076c1e308b51cde29c700ce71ce11a Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 10 Mar 2020 22:26:45 +0000 Subject: [PATCH 02/12] . --- src/consensus/pbft/libbyz/Big_req_table.cpp | 55 +++++----------- src/consensus/pbft/libbyz/Big_req_table.h | 33 +++++++++- src/consensus/pbft/libbyz/Checkpoint.cpp | 14 +---- src/consensus/pbft/libbyz/Req_queue.cpp | 69 +++++++++------------ src/consensus/pbft/libbyz/Req_queue.h | 24 ++++--- 5 files changed, 93 insertions(+), 102 deletions(-) diff --git a/src/consensus/pbft/libbyz/Big_req_table.cpp b/src/consensus/pbft/libbyz/Big_req_table.cpp index a7d10bb0bd63..fce3167d849e 100644 --- a/src/consensus/pbft/libbyz/Big_req_table.cpp +++ b/src/consensus/pbft/libbyz/Big_req_table.cpp @@ -10,30 +10,6 @@ #include "Request.h" #include "ds/logger.h" -struct Waiting_pp -{ - Seqno n; - View v; - int i; -}; - -class BR_entry -{ -public: - inline BR_entry() : r(0), maxn(-1), maxv(-1) {} - inline ~BR_entry() - { - delete r; - } - - Digest rd; // Request's digest - Request* r; // Request or 0 is request not received - // if r=0, Seqnos of pre-prepares waiting for request - std::vector waiting; - Seqno maxn; // Maximum seqno of pre-prepare referencing request - View maxv; // Maximum view in which this entry was marked useful -}; - Big_req_table::Big_req_table(size_t num_of_replicas) : breqs(max_out), last_stable(0), @@ -67,7 +43,7 @@ inline void Big_req_table::remove_unmatched(BR_entry* bre) PBFT_ASSERT(bre->r != 0, "Invalid state"); auto& centry = unmatched[bre->r->client_id()]; - centry.requests.remove(bre->r); + centry.list.remove(bre); centry.num_requests--; PBFT_ASSERT(centry.num_requests >= 0, "Should be positive"); } @@ -204,35 +180,34 @@ bool Big_req_table::check_pcerts(BR_entry* bre) return false; } -bool Big_req_table::add_unmatched(Request* r, Request*& old_req) +bool Big_req_table::add_unmatched(BR_entry* e, Request*& old_req) { - auto& centry = unmatched[r->client_id()]; + auto& centry = unmatched[e->r->client_id()]; old_req = 0; if ( - !centry.requests.empty() && - centry.last_value_seen[r->user_id()] >= r->request_id()) + !centry.list.is_empty() && + centry.last_value_seen[e->r->user_id()] >= e->r->request_id()) { // client is expected to send requests in request id order LOG_FAIL_FMT( "client is expected to send requests in request id order {}", - r->client_id()); + e->r->client_id()); return false; } if (centry.num_requests >= Max_unmatched_requests_per_client) { - LOG_FAIL_FMT("Too many Requests pending from client: {}", r->client_id()); - old_req = centry.requests.back(); - centry.requests.pop_back(); + LOG_FAIL_FMT("Too many Requests pending from client: {}", e->r->client_id()); + old_req = centry.list.pop_tail()->r; } else { centry.num_requests++; } - centry.last_value_seen[r->user_id()] = r->request_id(); - centry.requests.push_front(r); + centry.last_value_seen[e->r->user_id()] = e->r->request_id(); + centry.list.insert(e); return true; } @@ -294,13 +269,13 @@ bool Big_req_table::add_request(Request* r, bool verified) // Buffer up to Max_unmatched_requests_per_client requests with the // largest timestamps from client. Request* old_req = 0; - bool added = add_unmatched(r, old_req); + auto bre = std::make_unique(); + bre->rd = rd; + bre->r = r; + bool added = add_unmatched(bre.get(), old_req); if (added) { - auto bre = new BR_entry; - bre->rd = rd; - bre->r = r; - breqs.insert({rd, bre}); + breqs.insert({rd, bre.release()}); if (old_req) { diff --git a/src/consensus/pbft/libbyz/Big_req_table.h b/src/consensus/pbft/libbyz/Big_req_table.h index 353f35577be4..b395a5d37ddd 100644 --- a/src/consensus/pbft/libbyz/Big_req_table.h +++ b/src/consensus/pbft/libbyz/Big_req_table.h @@ -8,15 +8,42 @@ #include "Digest.h" #include "Req_queue.h" #include "ds/thread_messaging.h" +#include "ds/dllist.h" #include "types.h" #include #include #include -class BR_entry; class Request; +struct Waiting_pp +{ + Seqno n; + View v; + int i; +}; + +class BR_entry +{ +public: + inline BR_entry() : r(0), maxn(-1), maxv(-1), next(nullptr), prev(nullptr) {} + inline ~BR_entry() + { + delete r; + } + + Digest rd; // Request's digest + Request* r; // Request or 0 is request not received + // if r=0, Seqnos of pre-prepares waiting for request + std::vector waiting; + Seqno maxn; // Maximum seqno of pre-prepare referencing request + View maxv; // Maximum view in which this entry was marked useful + + BR_entry* next; + BR_entry* prev; +}; + class Big_req_table { // @@ -98,7 +125,7 @@ class Big_req_table // Effects: Removes bre->r from unmatched if it was not previously matched // to a pre-prepare. - bool add_unmatched(Request* r, Request*& old_req); + bool add_unmatched(BR_entry* e, Request*& old_req); // Effects: Adds r to the list of requests for the client if the request // id is greater than the largest in the list and returns true. If this causes // the number of requests to exceed Max_unmatched_requests_per_client, @@ -114,7 +141,7 @@ class Big_req_table struct Unmatched_requests { Unmatched_requests() : num_requests(0) {} - std::list requests; + snmalloc::DLList list; int num_requests; std::array last_value_seen = {0}; diff --git a/src/consensus/pbft/libbyz/Checkpoint.cpp b/src/consensus/pbft/libbyz/Checkpoint.cpp index 6fd3f0372655..de3fb444915d 100644 --- a/src/consensus/pbft/libbyz/Checkpoint.cpp +++ b/src/consensus/pbft/libbyz/Checkpoint.cpp @@ -35,8 +35,7 @@ Checkpoint::Checkpoint(Seqno s, Digest& d, bool stable) : auth_len = sizeof(Checkpoint_rep); auth_src_offset = 0; #else - rep().sig_size = pbft::GlobalState::get_node().gen_signature( - contents(), sizeof(Checkpoint_rep), contents() + sizeof(Checkpoint_rep)); + rep().sig_size = 0; #endif } @@ -52,8 +51,7 @@ void Checkpoint::re_authenticate(Principal* p, bool stable) if (rep().extra != 1 && stable) { rep().extra = 1; - rep().sig_size = pbft::GlobalState::get_node().gen_signature( - contents(), sizeof(Checkpoint_rep), contents() + sizeof(Checkpoint_rep)); + rep().sig_size = 0; } #endif } @@ -66,14 +64,6 @@ bool Checkpoint::pre_verify() return false; } - // Check signature size. - if ( - size() - (int)sizeof(Checkpoint_rep) < - pbft::GlobalState::get_node().auth_size(id())) - { - return false; - } - return true; } diff --git a/src/consensus/pbft/libbyz/Req_queue.cpp b/src/consensus/pbft/libbyz/Req_queue.cpp index 08afd6dfeb3e..9301fcbf45f4 100644 --- a/src/consensus/pbft/libbyz/Req_queue.cpp +++ b/src/consensus/pbft/libbyz/Req_queue.cpp @@ -11,8 +11,6 @@ Req_queue::Req_queue() : reqs(Max_num_replicas), - head(nullptr), - tail(nullptr), nelems(0), nbytes(0) {} @@ -21,7 +19,7 @@ bool Req_queue::append(Request* r) { size_t cid = r->client_id(); Request_id rid = r->request_id(); - + int user_id = r->user_id(); auto it = reqs.find({cid, rid}); if (it == reqs.end()) { @@ -30,18 +28,7 @@ bool Req_queue::append(Request* r) auto rn = std::make_unique(); rn->r.reset(r); - if (head == nullptr) - { - head = tail = rn.get(); - rn->prev = rn->next = nullptr; - } - else - { - tail->next = rn.get(); - rn->prev = tail; - rn->next = nullptr; - tail = rn.get(); - } + rnodes[user_id].insert_back(rn.get()); reqs.insert({Key{cid, rid}, std::move(rn)}); return true; @@ -66,24 +53,29 @@ bool Req_queue::is_in_rqueue(Request* r) Request* Req_queue::remove() { - if (head == nullptr) - { - return nullptr; - } + uint32_t tcount = enclave::ThreadMessaging::thread_count; + tcount = std::max(tcount, (uint32_t)1); - Request* ret = head->r.release(); - PBFT_ASSERT(ret != 0, "Invalid state"); - - head = head->next; - if (head != nullptr) + bool found = false; + for (uint i = 0; i < tcount; ++i) { - head->prev = nullptr; + if (!rnodes[count % tcount].is_empty()) + { + found = true; + break; + } + count++; } - else + + if (!found) { - tail = nullptr; + return nullptr; } + auto rn = rnodes[count % tcount].pop(); + Request* ret = rn->r.release(); + PBFT_ASSERT(ret != 0, "Invalid state"); + nelems--; nbytes -= ret->size(); @@ -108,8 +100,8 @@ bool Req_queue::remove(int cid, Request_id rid) bool ret = false; if (rn->prev == nullptr) { - PBFT_ASSERT(head == rn.get(), "Invalid state"); - head = rn->next; + PBFT_ASSERT(rnodes[rn->r->user_id()].get_head() == rn.get(), "Invalid state"); + rnodes[rn->r->user_id()].remove(rn.get()); ret = true; } else @@ -117,16 +109,6 @@ bool Req_queue::remove(int cid, Request_id rid) rn->prev->next = rn->next; } - if (rn->next == nullptr) - { - PBFT_ASSERT(tail == rn.get(), "Invalid state"); - tail = rn->prev; - } - else - { - rn->next->prev = rn->prev; - } - reqs.erase(it); return ret; @@ -134,8 +116,15 @@ bool Req_queue::remove(int cid, Request_id rid) void Req_queue::clear() { + uint32_t tcount = enclave::ThreadMessaging::thread_count; + tcount = std::max(tcount, (uint32_t)1); reqs.clear(); - head = tail = nullptr; + for (uint32_t i = 0; i < tcount; ++i) + { + while(!rnodes[i].is_empty()){ + rnodes[i].pop(); + } + } nelems = nbytes = 0; } diff --git a/src/consensus/pbft/libbyz/Req_queue.h b/src/consensus/pbft/libbyz/Req_queue.h index ca9cac69c647..bed92f004d77 100644 --- a/src/consensus/pbft/libbyz/Req_queue.h +++ b/src/consensus/pbft/libbyz/Req_queue.h @@ -8,8 +8,11 @@ #include "Request.h" #include "pbft_assert.h" #include "types.h" +#include "ds/thread_messaging.h" +#include "ds/dllist.h" #include +#include class Req_queue { @@ -83,9 +86,8 @@ class Req_queue } }; std::unordered_map, KeyHash> reqs; - - RNode* head; - RNode* tail; + mutable snmalloc::DLList rnodes[enclave::ThreadMessaging::max_num_threads]; + mutable uint64_t count = 0; int nelems; // Number of elements in queue int nbytes; // Number of bytes in queue @@ -103,10 +105,18 @@ inline int Req_queue::num_bytes() const inline Request* Req_queue::first() const { - if (head) + uint32_t tcount = enclave::ThreadMessaging::thread_count; + tcount = std::max(tcount, (uint32_t)1); + + for (uint32_t i = 0; i < tcount; ++i) { - PBFT_ASSERT(head->r != 0, "Invalid state"); - return head->r.get(); + count++; + auto rn = rnodes[count % tcount].get_head(); + if (rn != nullptr) + { + PBFT_ASSERT(rn->r != 0, "Invalid state"); + return rn->r.get(); + } } return 0; -} +} \ No newline at end of file From dbabdbffd7fed574235039bc109ae08a09ea4649 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 10 Mar 2020 22:40:10 +0000 Subject: [PATCH 03/12] print time series when collecting metrics --- src/node/rpc/metrics.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/node/rpc/metrics.h b/src/node/rpc/metrics.h index 3ac42ba3d8d3..033fb0b81020 100644 --- a/src/node/rpc/metrics.h +++ b/src/node/rpc/metrics.h @@ -60,6 +60,13 @@ namespace metrics result[std::to_string(i)]["duration"] = tx_time_passed[i]; } } + + LOG_INFO << "Printing time series" << std::endl; + for (uint32_t i = 0; i < times.size(); ++i) + { + LOG_INFO << i << " - " << times[i] << std::endl; + } + return result; } @@ -73,6 +80,8 @@ namespace metrics return result; } + std::array times = {0}; + void track_tx_rates( const std::chrono::milliseconds& elapsed, size_t tx_count) { @@ -91,6 +100,12 @@ namespace metrics tx_time_passed[tick_count] = rate_duration; } tick_count++; + + uint32_t bucket = rate_time_elapsed.count() / 1000.0; + if (bucket < times.size()) + { + times[bucket] += tx_count; + } } } }; From 22d4631f0ed41ba84aefb342da358e609d077a02 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 10 Mar 2020 22:42:36 +0000 Subject: [PATCH 04/12] stuff --- src/node/rpc/metrics.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node/rpc/metrics.h b/src/node/rpc/metrics.h index 033fb0b81020..5f580179254c 100644 --- a/src/node/rpc/metrics.h +++ b/src/node/rpc/metrics.h @@ -27,6 +27,8 @@ namespace metrics histogram::Global global = histogram::Global("histogram", __FILE__, __LINE__); Hist histogram = Hist(global); + std::array times = {0}; + ccf::GetMetrics::HistogramResults get_histogram_results() { @@ -80,8 +82,6 @@ namespace metrics return result; } - std::array times = {0}; - void track_tx_rates( const std::chrono::milliseconds& elapsed, size_t tx_count) { From 89e96a8fe2af6fa9c83fe602b5b33107bb4ccef4 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 10 Mar 2020 22:55:04 +0000 Subject: [PATCH 05/12] . --- src/consensus/pbft/libbyz/Big_req_table.cpp | 3 ++- src/consensus/pbft/libbyz/Big_req_table.h | 2 +- src/consensus/pbft/libbyz/Req_queue.cpp | 24 ++++++++++----------- src/consensus/pbft/libbyz/Req_queue.h | 9 ++++---- src/ds/dllist.h | 12 ++++++----- src/node/rpc/metrics.h | 1 - 6 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/consensus/pbft/libbyz/Big_req_table.cpp b/src/consensus/pbft/libbyz/Big_req_table.cpp index fce3167d849e..ce8a1d7c4953 100644 --- a/src/consensus/pbft/libbyz/Big_req_table.cpp +++ b/src/consensus/pbft/libbyz/Big_req_table.cpp @@ -198,7 +198,8 @@ bool Big_req_table::add_unmatched(BR_entry* e, Request*& old_req) if (centry.num_requests >= Max_unmatched_requests_per_client) { - LOG_FAIL_FMT("Too many Requests pending from client: {}", e->r->client_id()); + LOG_FAIL_FMT( + "Too many Requests pending from client: {}", e->r->client_id()); old_req = centry.list.pop_tail()->r; } else diff --git a/src/consensus/pbft/libbyz/Big_req_table.h b/src/consensus/pbft/libbyz/Big_req_table.h index b395a5d37ddd..4bdeabe59ac5 100644 --- a/src/consensus/pbft/libbyz/Big_req_table.h +++ b/src/consensus/pbft/libbyz/Big_req_table.h @@ -7,8 +7,8 @@ #include "Digest.h" #include "Req_queue.h" -#include "ds/thread_messaging.h" #include "ds/dllist.h" +#include "ds/thread_messaging.h" #include "types.h" #include diff --git a/src/consensus/pbft/libbyz/Req_queue.cpp b/src/consensus/pbft/libbyz/Req_queue.cpp index 9301fcbf45f4..5fbe211ab4d0 100644 --- a/src/consensus/pbft/libbyz/Req_queue.cpp +++ b/src/consensus/pbft/libbyz/Req_queue.cpp @@ -9,11 +9,7 @@ #include "Pre_prepare.h" #include "Request.h" -Req_queue::Req_queue() : - reqs(Max_num_replicas), - nelems(0), - nbytes(0) -{} +Req_queue::Req_queue() : reqs(Max_num_replicas), nelems(0), nbytes(0) {} bool Req_queue::append(Request* r) { @@ -57,13 +53,13 @@ Request* Req_queue::remove() tcount = std::max(tcount, (uint32_t)1); bool found = false; - for (uint i = 0; i < tcount; ++i) + for (uint32_t i = 0; i < tcount; ++i) { - if (!rnodes[count % tcount].is_empty()) - { - found = true; - break; - } + if (!rnodes[count % tcount].is_empty()) + { + found = true; + break; + } count++; } @@ -100,7 +96,8 @@ bool Req_queue::remove(int cid, Request_id rid) bool ret = false; if (rn->prev == nullptr) { - PBFT_ASSERT(rnodes[rn->r->user_id()].get_head() == rn.get(), "Invalid state"); + PBFT_ASSERT( + rnodes[rn->r->user_id()].get_head() == rn.get(), "Invalid state"); rnodes[rn->r->user_id()].remove(rn.get()); ret = true; } @@ -121,7 +118,8 @@ void Req_queue::clear() reqs.clear(); for (uint32_t i = 0; i < tcount; ++i) { - while(!rnodes[i].is_empty()){ + while (!rnodes[i].is_empty()) + { rnodes[i].pop(); } } diff --git a/src/consensus/pbft/libbyz/Req_queue.h b/src/consensus/pbft/libbyz/Req_queue.h index bed92f004d77..af00db340648 100644 --- a/src/consensus/pbft/libbyz/Req_queue.h +++ b/src/consensus/pbft/libbyz/Req_queue.h @@ -6,13 +6,13 @@ #pragma once #include "Request.h" +#include "ds/dllist.h" +#include "ds/thread_messaging.h" #include "pbft_assert.h" #include "types.h" -#include "ds/thread_messaging.h" -#include "ds/dllist.h" -#include #include +#include class Req_queue { @@ -86,7 +86,8 @@ class Req_queue } }; std::unordered_map, KeyHash> reqs; - mutable snmalloc::DLList rnodes[enclave::ThreadMessaging::max_num_threads]; + mutable snmalloc::DLList + rnodes[enclave::ThreadMessaging::max_num_threads]; mutable uint64_t count = 0; int nelems; // Number of elements in queue diff --git a/src/ds/dllist.h b/src/ds/dllist.h index 57126cf7f91f..7d07b8dcd4d4 100644 --- a/src/ds/dllist.h +++ b/src/ds/dllist.h @@ -1,3 +1,5 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the Apache 2.0 License. #pragma once #include @@ -19,7 +21,7 @@ namespace snmalloc * Invalid pointer class. This is similar to `std::nullptr_t`, but allows * other values. */ - template + template struct InvalidPointer { /** @@ -27,7 +29,7 @@ namespace snmalloc * are always the same, invalid pointer values with different sentinels are * always different. */ - template + template constexpr bool operator==(const InvalidPointer&) { return Sentinel == OtherSentinel; @@ -37,7 +39,7 @@ namespace snmalloc * are always the same, invalid pointer values with different sentinels are * always different. */ - template + template constexpr bool operator!=(const InvalidPointer&) { return Sentinel != OtherSentinel; @@ -48,7 +50,7 @@ namespace snmalloc * provenance-free integer and so will trap if dereferenced, on other * systems the sentinel should be a value in unmapped memory. */ - template + template operator T*() const { return reinterpret_cast(Sentinel); @@ -62,7 +64,7 @@ namespace snmalloc } }; - template + template class DLList { private: diff --git a/src/node/rpc/metrics.h b/src/node/rpc/metrics.h index 5f580179254c..2a545d1deb05 100644 --- a/src/node/rpc/metrics.h +++ b/src/node/rpc/metrics.h @@ -29,7 +29,6 @@ namespace metrics Hist histogram = Hist(global); std::array times = {0}; - ccf::GetMetrics::HistogramResults get_histogram_results() { ccf::GetMetrics::HistogramResults result; From d080afe50e48d47ae88e369be9d26e6d8a4e9dd0 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 11 Mar 2020 00:10:48 +0000 Subject: [PATCH 06/12] fix --- src/consensus/pbft/libbyz/Req_queue.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/consensus/pbft/libbyz/Req_queue.cpp b/src/consensus/pbft/libbyz/Req_queue.cpp index 5fbe211ab4d0..807ddfc6d2bf 100644 --- a/src/consensus/pbft/libbyz/Req_queue.cpp +++ b/src/consensus/pbft/libbyz/Req_queue.cpp @@ -123,6 +123,7 @@ void Req_queue::clear() rnodes[i].pop(); } } + reqs.clear(); nelems = nbytes = 0; } From 2158e26b58e0a425523a8b7f97669de9bcf3fce2 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 11 Mar 2020 00:11:43 +0000 Subject: [PATCH 07/12] . --- src/consensus/pbft/libbyz/Req_queue.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/consensus/pbft/libbyz/Req_queue.cpp b/src/consensus/pbft/libbyz/Req_queue.cpp index 807ddfc6d2bf..d6c6d152ebd6 100644 --- a/src/consensus/pbft/libbyz/Req_queue.cpp +++ b/src/consensus/pbft/libbyz/Req_queue.cpp @@ -115,7 +115,6 @@ void Req_queue::clear() { uint32_t tcount = enclave::ThreadMessaging::thread_count; tcount = std::max(tcount, (uint32_t)1); - reqs.clear(); for (uint32_t i = 0; i < tcount; ++i) { while (!rnodes[i].is_empty()) From 53b06de1a758f2038f743a9dd28de46aa16b8291 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 11 Mar 2020 01:10:55 +0000 Subject: [PATCH 08/12] . --- src/ds/dllist.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/ds/dllist.h b/src/ds/dllist.h index 7d07b8dcd4d4..b88427e7dcda 100644 --- a/src/ds/dllist.h +++ b/src/ds/dllist.h @@ -64,7 +64,10 @@ namespace snmalloc } }; - template + template < + class T, + class Terminator = std::nullptr_t, + bool delete_on_clear = false> class DLList { private: @@ -205,7 +208,10 @@ namespace snmalloc { auto c = head; remove(c); - delete c; + if (delete_on_clear) + { + delete c; + } } } From 7927866c19a772d653e8fd64330b1a70fd2c11ba Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 11 Mar 2020 02:05:32 +0000 Subject: [PATCH 09/12] fix things, hopefully --- src/consensus/pbft/libbyz/Big_req_table.cpp | 16 +++++++++++----- src/consensus/pbft/libbyz/Replica.cpp | 6 ++++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/consensus/pbft/libbyz/Big_req_table.cpp b/src/consensus/pbft/libbyz/Big_req_table.cpp index ce8a1d7c4953..52d680c9a0e7 100644 --- a/src/consensus/pbft/libbyz/Big_req_table.cpp +++ b/src/consensus/pbft/libbyz/Big_req_table.cpp @@ -191,8 +191,9 @@ bool Big_req_table::add_unmatched(BR_entry* e, Request*& old_req) { // client is expected to send requests in request id order LOG_FAIL_FMT( - "client is expected to send requests in request id order {}", - e->r->client_id()); + "client is expected to send requests in request id order {}, last seen {}", + e->r->client_id(), + centry.last_value_seen[e->r->user_id()]); return false; } @@ -270,13 +271,13 @@ bool Big_req_table::add_request(Request* r, bool verified) // Buffer up to Max_unmatched_requests_per_client requests with the // largest timestamps from client. Request* old_req = 0; - auto bre = std::make_unique(); + auto bre = new BR_entry(); bre->rd = rd; bre->r = r; - bool added = add_unmatched(bre.get(), old_req); + bool added = add_unmatched(bre, old_req); if (added) { - breqs.insert({rd, bre.release()}); + breqs.insert({rd, bre}); if (old_req) { @@ -289,6 +290,11 @@ bool Big_req_table::add_request(Request* r, bool verified) return true; } + else + { + bre->r = nullptr; + delete bre; + } } return false; } diff --git a/src/consensus/pbft/libbyz/Replica.cpp b/src/consensus/pbft/libbyz/Replica.cpp index 4acfb3faf135..3208ab95b6c1 100644 --- a/src/consensus/pbft/libbyz/Replica.cpp +++ b/src/consensus/pbft/libbyz/Replica.cpp @@ -313,6 +313,7 @@ Message* Replica::create_message(const uint8_t* data, uint32_t size) default: // Unknown message type. + LOG_FAIL << "Unknown message type:" << Message::get_tag(data) << std::endl; delete m; return nullptr; } @@ -329,6 +330,11 @@ void Replica::receive_message(const uint8_t* data, uint32_t size) LOG_FAIL << "Received message size exceeds message: " << size << std::endl; } Message* m = create_message(data, size); + if (m == nullptr) + { + return; + } + uint32_t target_thread = 0; if (enclave::ThreadMessaging::thread_count > 1 && m->tag() == Request_tag) From dca5876b394a4b36b0cfef481655722620a8e414 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 11 Mar 2020 02:10:23 +0000 Subject: [PATCH 10/12] . --- src/consensus/pbft/libbyz/Big_req_table.cpp | 3 ++- src/consensus/pbft/libbyz/Replica.cpp | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/consensus/pbft/libbyz/Big_req_table.cpp b/src/consensus/pbft/libbyz/Big_req_table.cpp index 52d680c9a0e7..8b08eed638f2 100644 --- a/src/consensus/pbft/libbyz/Big_req_table.cpp +++ b/src/consensus/pbft/libbyz/Big_req_table.cpp @@ -191,7 +191,8 @@ bool Big_req_table::add_unmatched(BR_entry* e, Request*& old_req) { // client is expected to send requests in request id order LOG_FAIL_FMT( - "client is expected to send requests in request id order {}, last seen {}", + "client is expected to send requests in request id order {}, last seen " + "{}", e->r->client_id(), centry.last_value_seen[e->r->user_id()]); return false; diff --git a/src/consensus/pbft/libbyz/Replica.cpp b/src/consensus/pbft/libbyz/Replica.cpp index 3208ab95b6c1..d3bedbde612b 100644 --- a/src/consensus/pbft/libbyz/Replica.cpp +++ b/src/consensus/pbft/libbyz/Replica.cpp @@ -313,7 +313,7 @@ Message* Replica::create_message(const uint8_t* data, uint32_t size) default: // Unknown message type. - LOG_FAIL << "Unknown message type:" << Message::get_tag(data) << std::endl; + LOG_FAIL_FMT("Unknown message type:{}", Message::get_tag(data)); delete m; return nullptr; } @@ -330,7 +330,7 @@ void Replica::receive_message(const uint8_t* data, uint32_t size) LOG_FAIL << "Received message size exceeds message: " << size << std::endl; } Message* m = create_message(data, size); - if (m == nullptr) + if (m == nullptr) { return; } From 5ad2b9dac2a3276235a1fba7cb0fe1dfef8e7dc2 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 11 Mar 2020 09:24:13 +0000 Subject: [PATCH 11/12] . --- src/ds/dllist.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ds/dllist.h b/src/ds/dllist.h index b88427e7dcda..137145e449f4 100644 --- a/src/ds/dllist.h +++ b/src/ds/dllist.h @@ -68,7 +68,7 @@ namespace snmalloc class T, class Terminator = std::nullptr_t, bool delete_on_clear = false> - class DLList + class DLList final { private: static_assert( @@ -80,7 +80,7 @@ namespace snmalloc T* tail = Terminator(); public: - virtual ~DLList() + ~DLList() { clear(); } From 15a489ee4d987e472882a4ec6b43ae5a5d697307 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 11 Mar 2020 10:45:50 +0000 Subject: [PATCH 12/12] . --- src/consensus/pbft/libbyz/Req_queue.cpp | 2 ++ src/node/rpc/metrics.h | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/consensus/pbft/libbyz/Req_queue.cpp b/src/consensus/pbft/libbyz/Req_queue.cpp index d6c6d152ebd6..93c6f13a0155 100644 --- a/src/consensus/pbft/libbyz/Req_queue.cpp +++ b/src/consensus/pbft/libbyz/Req_queue.cpp @@ -114,6 +114,8 @@ bool Req_queue::remove(int cid, Request_id rid) void Req_queue::clear() { uint32_t tcount = enclave::ThreadMessaging::thread_count; + // There is a corner case when we run the very first transaction that + // thread_count can be 0. The use of std::max is a work around. tcount = std::max(tcount, (uint32_t)1); for (uint32_t i = 0; i < tcount; ++i) { diff --git a/src/node/rpc/metrics.h b/src/node/rpc/metrics.h index 2a545d1deb05..24bdbc297517 100644 --- a/src/node/rpc/metrics.h +++ b/src/node/rpc/metrics.h @@ -65,7 +65,7 @@ namespace metrics LOG_INFO << "Printing time series" << std::endl; for (uint32_t i = 0; i < times.size(); ++i) { - LOG_INFO << i << " - " << times[i] << std::endl; + LOG_INFO_FMT("{} - {}", i, times[i]); } return result;