From 83b0f9d0107ddace64c7742cdcf46aa1258550b2 Mon Sep 17 00:00:00 2001 From: olgavrou Date: Fri, 28 Feb 2020 17:06:55 +0000 Subject: [PATCH] Dont remove pending requests from big request table (#899) --- src/consensus/pbft/libbyz/Big_req_table.cpp | 16 +++++++++++++++- src/consensus/pbft/libbyz/Big_req_table.h | 5 +++-- src/consensus/pbft/libbyz/Replica.cpp | 2 +- src/consensus/pbft/libbyz/Req_queue.cpp | 13 +++++++++++++ src/consensus/pbft/libbyz/Req_queue.h | 3 +++ 5 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/consensus/pbft/libbyz/Big_req_table.cpp b/src/consensus/pbft/libbyz/Big_req_table.cpp index c6267081c9..a7d10bb0bd 100644 --- a/src/consensus/pbft/libbyz/Big_req_table.cpp +++ b/src/consensus/pbft/libbyz/Big_req_table.cpp @@ -339,7 +339,7 @@ void Big_req_table::clear() } } -void Big_req_table::mark_stable(Seqno ls) +void Big_req_table::mark_stable(Seqno ls, Req_queue& rqueue) { last_stable = ls; @@ -348,6 +348,20 @@ void Big_req_table::mark_stable(Seqno ls) auto bre = it->second; if (bre->maxn <= ls && bre->maxv >= 0) { + if (bre->maxn < 0) + { + PBFT_ASSERT(bre->r != 0, "Invalid state"); + if (rqueue.is_in_rqueue(bre->r)) + { + LOG_TRACE_FMT( + "Request is in rqueue don't remove it from big req table {} {} {}", + bre->r->request_id(), + bre->r->client_id(), + bre->r->digest().hash()); + it++; + continue; + } + } remove_unmatched(bre); delete bre; it = breqs.erase(it); diff --git a/src/consensus/pbft/libbyz/Big_req_table.h b/src/consensus/pbft/libbyz/Big_req_table.h index 9bf618cc3c..353f35577b 100644 --- a/src/consensus/pbft/libbyz/Big_req_table.h +++ b/src/consensus/pbft/libbyz/Big_req_table.h @@ -6,6 +6,7 @@ #pragma once #include "Digest.h" +#include "Req_queue.h" #include "ds/thread_messaging.h" #include "types.h" @@ -68,10 +69,10 @@ public: void clear(); // Effects: Discards (deletes) all stored entries - void mark_stable(Seqno ls); + void mark_stable(Seqno ls, Req_queue& req_queue); // Effects: Discards entries that were only referred to by // pre-prepares that were discarded due to checkpoint "ls" becoming - // stable. + // stable. If an entry is in the req_queue it will not be removed. void view_change(View v); // Effects: Discards entries that were only referred to by diff --git a/src/consensus/pbft/libbyz/Replica.cpp b/src/consensus/pbft/libbyz/Replica.cpp index 175a2692aa..e8743c0a02 100644 --- a/src/consensus/pbft/libbyz/Replica.cpp +++ b/src/consensus/pbft/libbyz/Replica.cpp @@ -2634,7 +2634,7 @@ void Replica::mark_stable(Seqno n, bool have_state) vi.mark_stable(last_stable); elog.truncate(last_stable); state.discard_checkpoints(last_stable, last_executed); - brt.mark_stable(last_stable); + brt.mark_stable(last_stable, rqueue); if (mark_stable_cb != nullptr) { diff --git a/src/consensus/pbft/libbyz/Req_queue.cpp b/src/consensus/pbft/libbyz/Req_queue.cpp index b74c7514cf..08afd6dfeb 100644 --- a/src/consensus/pbft/libbyz/Req_queue.cpp +++ b/src/consensus/pbft/libbyz/Req_queue.cpp @@ -51,6 +51,19 @@ bool Req_queue::append(Request* r) return false; } +bool Req_queue::is_in_rqueue(Request* r) +{ + size_t cid = r->client_id(); + Request_id rid = r->request_id(); + + auto it = reqs.find({cid, rid}); + if (it == reqs.end()) + { + return false; + } + return true; +} + Request* Req_queue::remove() { if (head == nullptr) diff --git a/src/consensus/pbft/libbyz/Req_queue.h b/src/consensus/pbft/libbyz/Req_queue.h index 79d9e556f8..ca9cac69c6 100644 --- a/src/consensus/pbft/libbyz/Req_queue.h +++ b/src/consensus/pbft/libbyz/Req_queue.h @@ -27,6 +27,9 @@ public: // other request from "r->client_id()" from the queue and returns // true. Otherwise, returns false. + bool is_in_rqueue(Request* r); + // Effects: returns true if the request is in the rqueue + Request* remove(); // Effects: If there is any element in the queue, removes the first // element in the queue and returns it. Otherwise, returns 0.