Skip to content

Commit 802f061

Browse files
authored
Switch trigger timer on p28 boundary (#5350)
# Description This enables the new trigger timer on the protocol 28 boundary. While we had discussed the potential for this to be a non-protocol upgrade, and just ask operators to switch at around the same time, we've received slow response times historically from tier 1, which could leave the network in a degraded state. This is the safest way to land the change. I've also added a flag that, when set, forces validators to use the old timer after the p28 upgrade. In the event that we see an unexpected perf issue after the upgrade, we can instruct operators to set this flag to roll back the change without a protocol upgrade. Finally, I also added an upgrade path test, and fixed some flakey asserts. Specifically, in the node drift tests, we assert that the drifting node detects it's drift and falls back to the old timer. However, if the drifting node's timer is externalized as the closeTime, the non-drifting nodes fall back, as they think they are drifting relative to the rest of the network. This is safe behavior, but the test would sometimes fail based on leader schedules. # Checklist - [x] Reviewed the [contributing](https://github.com/stellar/stellar-core/blob/master/CONTRIBUTING.md#submitting-changes) document - [x] Rebased on top of master (no merge commits) - [x] Ran `clang-format` v8.0.0 (via `make format` or the Visual Studio extension) - [x] Compiles - [x] Ran all tests - [ ] If change impacts performance, include supporting evidence per the [performance document](https://github.com/stellar/stellar-core/blob/master/performance-eval/performance-eval.md)
2 parents 7b943ae + ea6eecf commit 802f061

8 files changed

Lines changed: 297 additions & 77 deletions

File tree

docs/metrics.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ scp.timing.externalized | timer | time spent in ballot pro
201201
scp.timing.first-to-self-externalize-lag | timer | delay between first externalize message and local node externalizing
202202
scp.timing.self-to-others-externalize-lag | timer | delay between local node externalizing and later externalize messages from other nodes
203203
scp.timing.ballot-blocked-on-txset | timer | time balloting was blocked waiting for a txset download (milliseconds)
204-
scp.trigger.prepare-start-fallback | meter | experimental trigger timer fell back from the network-close-time anchor to the local prepare-start anchor
204+
scp.trigger.prepare-start-fallback | meter | trigger timer fell back from the network-close-time anchor to the local prepare-start anchor
205205
scp.value.invalid | meter | SCP value is invalid
206206
scp.value.valid | meter | SCP value is valid
207207
scp.slot.values-referenced | histogram | number of values referenced per consensus round

src/herder/HerderImpl.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1447,8 +1447,18 @@ HerderImpl::setupTriggerNextLedger()
14471447

14481448
auto now = mApp.getClock().now();
14491449

1450+
// Starting from protocol 28 the trigger timer is anchored on the
1451+
// network-agreed consensus close time.
1452+
// FORCE_OLD_STYLE_PREPARE_START_TRIGGER_TIMER forces the older
1453+
// prepare-start anchor as an emergency fallback.
1454+
bool const useConsensusCloseTimeAnchor =
1455+
protocolVersionStartsFrom(
1456+
lcl.header.ledgerVersion,
1457+
CONSENSUS_CLOSE_TIME_TRIGGER_PROTOCOL_VERSION) &&
1458+
!mApp.getConfig().FORCE_OLD_STYLE_PREPARE_START_TRIGGER_TIMER;
1459+
14501460
auto lastLedgerStartingPoint =
1451-
mApp.getConfig().EXPERIMENTAL_TRIGGER_TIMER
1461+
useConsensusCloseTimeAnchor
14521462
? triggerAnchorFromConsensusCloseTime(lastIndex, now, milliseconds)
14531463
: triggerAnchorFromPrepareStart(lastIndex, now, milliseconds);
14541464

src/herder/HerderImpl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ class HerderImpl : public Herder
358358
medida::Meter& mEnvelopeValidSig;
359359
medida::Meter& mEnvelopeInvalidSig;
360360

361-
// Marked when the experimental trigger timer falls back from the
361+
// Marked when the trigger timer falls back from the
362362
// network-close-time anchor to the local prepare-start anchor.
363363
medida::Meter& mTriggerPrepareStartFallback;
364364

0 commit comments

Comments
 (0)