diff --git a/agreement/player.go b/agreement/player.go index 9512f22bf1..7b292fda31 100644 --- a/agreement/player.go +++ b/agreement/player.go @@ -295,45 +295,35 @@ func (p *player) updateDynamicLambdaTimings(r routerHandle, ver protocol.Consens if re.Vote.validatedAt != 0 { p.lowestCredentialArrivals = append(p.lowestCredentialArrivals, re.Vote.validatedAt) } - p.resizeLowestCredentialArrivals(ver) -} -func (p *player) resizeLowestCredentialArrivals(ver protocol.ConsensusVersion) { + // trim history to the last proto.DynamicFilterCredentialArrivalHistory samples. proto := config.Consensus[ver] if len(p.lowestCredentialArrivals) > proto.DynamicFilterCredentialArrivalHistory { p.lowestCredentialArrivals = p.lowestCredentialArrivals[len(p.lowestCredentialArrivals)-proto.DynamicFilterCredentialArrivalHistory:] } - for len(p.lowestCredentialArrivals) < proto.DynamicFilterCredentialArrivalHistory { - p.lowestCredentialArrivals = append([]time.Duration{FilterTimeout(0, ver)}, p.lowestCredentialArrivals...) - } } // calculateFilterTimeout chooses the appropriate filter timeout. func (p *player) calculateFilterTimeout(ver protocol.ConsensusVersion, tracer *tracer) time.Duration { proto := config.Consensus[ver] - - if !proto.DynamicFilterTimeout || p.Period != 0 { + if proto.DynamicFilterCredentialArrivalHistory <= 0 || p.Period != 0 { // Either dynamic lambda is disabled, or we're not in period 0 and // therefore, can't use dynamic lambda return FilterTimeout(p.Period, ver) } - var dynamicDelay time.Duration defaultDelay := FilterTimeout(0, ver) - if proto.DynamicFilterCredentialArrivalHistory <= 0 { - // we don't keep any history, use the default - dynamicDelay = defaultDelay - } else if proto.DynamicFilterCredentialArrivalHistory > len(p.lowestCredentialArrivals) { + if proto.DynamicFilterCredentialArrivalHistory > len(p.lowestCredentialArrivals) { // not enough samples, use the default - dynamicDelay = defaultDelay - } else { - sortedArrivals := make([]time.Duration, len(p.lowestCredentialArrivals)) - copy(sortedArrivals[:], p.lowestCredentialArrivals[:]) - sort.Slice(sortedArrivals, func(i, j int) bool { return sortedArrivals[i] < sortedArrivals[j] }) - dynamicDelay = sortedArrivals[proto.DynamicFilterCredentialArrivalHistoryIdx] + 50*time.Millisecond + return defaultDelay } + sortedArrivals := make([]time.Duration, len(p.lowestCredentialArrivals)) + copy(sortedArrivals[:], p.lowestCredentialArrivals[:]) + sort.Slice(sortedArrivals, func(i, j int) bool { return sortedArrivals[i] < sortedArrivals[j] }) + dynamicDelay := sortedArrivals[proto.DynamicFilterCredentialArrivalHistoryIdx] + proto.DynamicFilterGraceTime + // Make sure the dynamic delay is not too small or too large if dynamicDelay < proto.DynamicFilterTimeoutLowerBound { if tracer != nil { @@ -341,8 +331,10 @@ func (p *player) calculateFilterTimeout(ver protocol.ConsensusVersion, tracer *t } dynamicDelay = proto.DynamicFilterTimeoutLowerBound } else if dynamicDelay > defaultDelay { + if tracer != nil { + tracer.log.Debugf("Calculated dynamic delay = %v for round %v, period %v. It's higher than the default %v\n", dynamicDelay, p.Round, p.Period, defaultDelay) + } dynamicDelay = defaultDelay - tracer.log.Warnf("Calculated dynamic delay = %v for round %v, period %v. It's higher than the default %v\n", dynamicDelay, p.Round, p.Period, defaultDelay) } else if tracer != nil { tracer.log.Debugf("Calculated dynamic delay = %v for round %v, period %v.\n", dynamicDelay, p.Round, p.Period) } diff --git a/agreement/player_test.go b/agreement/player_test.go index b9f950a555..4d1a971272 100644 --- a/agreement/player_test.go +++ b/agreement/player_test.go @@ -3236,7 +3236,7 @@ func TestPlayerAlwaysResynchsPinnedValue(t *testing.T) { // test that ReceivedAt and ValidateAt timing information are retained in proposalStore // when the payloadPresent, payloadVerified, and voteVerified events are processed, and that all timings // are available when the ensureAction is called for the block. -func TestPlayerRetainsReceivedValidatedAt(t *testing.T) { +func TestPlayerRetainsReceivedValidatedAtOneSample(t *testing.T) { partitiontest.PartitionTest(t) const r = round(20239) @@ -3265,15 +3265,15 @@ func TestPlayerRetainsReceivedValidatedAt(t *testing.T) { // assert lowest vote validateAt time was recorded into payloadArrivals historyLen := config.Consensus[protocol.ConsensusFuture].DynamicFilterCredentialArrivalHistory require.NotZero(t, historyLen) - require.Len(t, pWhite.lowestCredentialArrivals, historyLen) - require.Equal(t, 501*time.Millisecond, pWhite.lowestCredentialArrivals[historyLen-1]) + require.Len(t, pWhite.lowestCredentialArrivals, 1) + require.Equal(t, 501*time.Millisecond, pWhite.lowestCredentialArrivals[0]) } // test that ReceivedAt and ValidateAt timing information are retained in proposalStore // when the payloadPresent (as part of the CompoundMessage encoding used by PP messages), // payloadVerified, and voteVerified events are processed, and that all timings // are available when the ensureAction is called for the block. -func TestPlayerRetainsReceivedValidatedAtPP(t *testing.T) { +func TestPlayerRetainsReceivedValidatedAtPPOneSample(t *testing.T) { partitiontest.PartitionTest(t) const r = round(20239) @@ -3309,8 +3309,8 @@ func TestPlayerRetainsReceivedValidatedAtPP(t *testing.T) { // assert lowest vote validateAt time was recorded into payloadArrivals historyLen := config.Consensus[protocol.ConsensusFuture].DynamicFilterCredentialArrivalHistory require.NotZero(t, historyLen) - require.Len(t, pWhite.lowestCredentialArrivals, historyLen) - require.Equal(t, 502*time.Millisecond, pWhite.lowestCredentialArrivals[historyLen-1]) + require.Len(t, pWhite.lowestCredentialArrivals, 1) + require.Equal(t, 502*time.Millisecond, pWhite.lowestCredentialArrivals[0]) } // test that ReceivedAt and ValidateAt timing information are retained in proposalStore @@ -3318,7 +3318,7 @@ func TestPlayerRetainsReceivedValidatedAtPP(t *testing.T) { // then the payloadPresent (as part of the CompoundMessage encoding used by PP messages) // and payloadVerified events are processed, and that all timings // are available when the ensureAction is called for the block. -func TestPlayerRetainsReceivedValidatedAtAVPP(t *testing.T) { +func TestPlayerRetainsReceivedValidatedAtAVPPOneSample(t *testing.T) { partitiontest.PartitionTest(t) const r = round(20239) @@ -3364,8 +3364,8 @@ func TestPlayerRetainsReceivedValidatedAtAVPP(t *testing.T) { // assert lowest vote validateAt time was recorded into payloadArrivals historyLen := config.Consensus[protocol.ConsensusFuture].DynamicFilterCredentialArrivalHistory require.NotZero(t, historyLen) - require.Len(t, pWhite.lowestCredentialArrivals, historyLen) - require.Equal(t, 502*time.Millisecond, pWhite.lowestCredentialArrivals[historyLen-1]) + require.Len(t, pWhite.lowestCredentialArrivals, 1) + require.Equal(t, 502*time.Millisecond, pWhite.lowestCredentialArrivals[0]) } func assertCorrectReceivedAtSet(t *testing.T, pWhite *player, pM ioAutomata, helper *voteMakerHelper, diff --git a/agreement/service_test.go b/agreement/service_test.go index f8822a6bfd..96977e812d 100644 --- a/agreement/service_test.go +++ b/agreement/service_test.go @@ -1002,7 +1002,7 @@ func TestAgreementSynchronousFuture5_DynamicFilterRounds(t *testing.T) { } cfg := config.Consensus[protocol.ConsensusFuture] - if !cfg.DynamicFilterTimeout { + if cfg.DynamicFilterCredentialArrivalHistory <= 0 { return } diff --git a/config/consensus.go b/config/consensus.go index f34ff896a6..79a2433ce6 100644 --- a/config/consensus.go +++ b/config/consensus.go @@ -517,21 +517,24 @@ type ConsensusParams struct { // by state proofs to use the same method (rather than excluding stake from the top N stakeholders as before). ExcludeExpiredCirculation bool - // DynamicFilterTimeout specifies whether the timeout for the filter step - // should be determined dynamically, at run-time. - DynamicFilterTimeout bool + // DynamicFilterCredentialArrivalHistory specifies the number of past + // credential arrivals that are measured to determine the next filter + // timeout. If DynamicFilterCredentialArrivalHistory <= 0, then the dynamic + // timeout feature is off and the filter step timeout is calculated using + // the static configuration. + DynamicFilterCredentialArrivalHistory int // DynamicFilterTimeoutLowerBound specifies a minimal duration that the // filter timeout must meet. DynamicFilterTimeoutLowerBound time.Duration - // DynamicFilterCredentialArrivalHistory specifies the number of past credential arrivals - // that are measured to determine the next filter timeout. - DynamicFilterCredentialArrivalHistory int - // DynamicFilterCredentialArrivalHistoryIdx specified which sample to use out of a sorted // DynamicFilterCredentialArrivalHistory-sized array of time samples. DynamicFilterCredentialArrivalHistoryIdx int + + // DynamicFilterGraceTime is additional extention to the dynamic filter time atop the + // one calculated based on the history of credential arrivals. + DynamicFilterGraceTime time.Duration } // PaysetCommitType enumerates possible ways for the block header to commit to @@ -1379,12 +1382,12 @@ func initConsensusProtocols() { vFuture.LogicSigVersion = 10 // When moving this to a release, put a new higher LogicSigVersion here vFuture.EnableLogicSigCostPooling = true - vFuture.DynamicFilterTimeout = true - vFuture.DynamicFilterTimeoutLowerBound = time.Second // history window of 40, so we have enough statistics to calculate the 95th // percentile, which is the timestamp at index 37 in the history array. vFuture.DynamicFilterCredentialArrivalHistory = 40 vFuture.DynamicFilterCredentialArrivalHistoryIdx = 37 + vFuture.DynamicFilterTimeoutLowerBound = time.Second + vFuture.DynamicFilterGraceTime = 50 * time.Millisecond Consensus[protocol.ConsensusFuture] = vFuture diff --git a/tools/x-repo-types/xrt_test.go b/tools/x-repo-types/xrt_test.go index 6fe353cedb..4360b432d6 100644 --- a/tools/x-repo-types/xrt_test.go +++ b/tools/x-repo-types/xrt_test.go @@ -74,14 +74,12 @@ func TestCrossRepoTypes(t *testing.T) { yType: "EvalDelta", }, { - name: "goal-v-sdk-consensus", - xPkg: "github.com/algorand/go-algorand/config", - xType: "ConsensusParams", - yPkg: "github.com/algorand/go-algorand-sdk/v2/protocol/config", - yBranch: "develop", - yType: "ConsensusParams", - skip: true, - skipReason: `Changes to consensus parameters happen on the go-algorand repo, and cannot be synced in a single PR with changes to the go-algorand-sdk repo`, + name: "goal-v-sdk-consensus", + xPkg: "github.com/algorand/go-algorand/config", + xType: "ConsensusParams", + yPkg: "github.com/algorand/go-algorand-sdk/v2/protocol/config", + yBranch: "develop", + yType: "ConsensusParams", }, { name: "goal-v-sdk-blockheader",