Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(op-node/op-batcher/op-proposer): the fallback client should always try recover #118

Merged
merged 11 commits into from
Mar 7, 2024
Merged

fix(op-node/op-batcher/op-proposer): the fallback client should always try recover #118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1aedca7
fix: the fallback client should always try recover
Jan 29, 2024
3276bd3
fix: don't close firstRpc
Jan 29, 2024
eac9d54
fix: also fix fallbackClient in batcher or proposer
Jan 30, 2024
2680520
Merge branch 'develop' into bugfix/fallback_recover
welkin22 Feb 1, 2024
c7631aa
add comments to trigger ci
welkin22 Mar 5, 2024
0c3dc69
lint
welkin22 Mar 5, 2024
4d6d88a
fix ci
welkin22 Mar 5, 2024
9233a21
fix ci
welkin22 Mar 5, 2024
19fb721
fix ci
welkin22 Mar 5, 2024
1c73cbc
fix ci
welkin22 Mar 5, 2024
cd9cebe
fix ci
welkin22 Mar 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 22 additions & 22 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ jobs:
uses: actions/checkout@v3

- name: Set up Go
uses: actions/setup-go@v2
uses: actions/setup-go@v5
with:
go-version-file: go.mod
go-version: '1.20.13'

- name: golangci-lint
uses: golangci/golangci-lint-action@v3
uses: golangci/golangci-lint-action@v4
with:
working-directory: op-node
version: latest
version: v1.55.2
args: -E goimports,sqlclosecheck,bodyclose,asciicheck,misspell,errorlint --timeout 5m -e "errors.As" -e "errors.Is"

op-batcher-lint:
Expand All @@ -34,15 +34,15 @@ jobs:
uses: actions/checkout@v3

- name: Set up Go
uses: actions/setup-go@v2
uses: actions/setup-go@v5
with:
go-version-file: go.mod
go-version: '1.20.13'

- name: golangci-lint
uses: golangci/golangci-lint-action@v3
uses: golangci/golangci-lint-action@v4
with:
working-directory: op-batcher
version: latest
version: v1.55.2
args: -E goimports,sqlclosecheck,bodyclose,asciicheck,misspell,errorlint --timeout 5m -e "errors.As" -e "errors.Is"

op-proposer-lint:
Expand All @@ -53,15 +53,15 @@ jobs:
uses: actions/checkout@v3

- name: Set up Go
uses: actions/setup-go@v2
uses: actions/setup-go@v5
with:
go-version-file: go.mod
go-version: '1.20.13'

- name: golangci-lint
uses: golangci/golangci-lint-action@v3
uses: golangci/golangci-lint-action@v4
with:
working-directory: op-proposer
version: latest
version: v1.55.2
args: -E goimports,sqlclosecheck,bodyclose,asciicheck,misspell,errorlint --timeout 5m -e "errors.As" -e "errors.Is"

op-node-test:
Expand All @@ -73,9 +73,9 @@ jobs:
uses: actions/checkout@v3

- name: Set up Go
uses: actions/setup-go@v2
uses: actions/setup-go@v5
with:
go-version-file: go.mod
go-version: '1.20.13'

- name: Install gotestsum
uses: autero1/action-gotestsum@v2.0.0
Expand All @@ -102,9 +102,9 @@ jobs:
uses: actions/checkout@v3

- name: Set up Go
uses: actions/setup-go@v2
uses: actions/setup-go@v5
with:
go-version-file: go.mod
go-version: '1.20.13'

- name: Install gotestsum
uses: autero1/action-gotestsum@v2.0.0
Expand All @@ -131,9 +131,9 @@ jobs:
uses: actions/checkout@v3

- name: Set up Go
uses: actions/setup-go@v2
uses: actions/setup-go@v5
with:
go-version-file: go.mod
go-version: '1.20.13'

- name: Install gotestsum
uses: autero1/action-gotestsum@v2.0.0
Expand All @@ -160,9 +160,9 @@ jobs:
uses: actions/checkout@v3

- name: Set up Go
uses: actions/setup-go@v2
uses: actions/setup-go@v5
with:
go-version-file: go.mod
go-version: '1.20.13'

- name: Install gotestsum
uses: autero1/action-gotestsum@v2.0.0
Expand Down Expand Up @@ -191,9 +191,9 @@ jobs:
uses: actions/checkout@v3

- name: Set up Go
uses: actions/setup-go@v2
uses: actions/setup-go@v5
with:
go-version-file: go.mod
go-version: '1.20.13'

- name: Install gotestsum
uses: autero1/action-gotestsum@v2.0.0
Expand Down
7 changes: 4 additions & 3 deletions op-e2e/actions/fallback_client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package actions

import (
"math/big"
"testing"
"time"

"github.com/ethereum-optimism/optimism/op-e2e/e2eutils"
"github.com/ethereum-optimism/optimism/op-node/client"
"github.com/ethereum-optimism/optimism/op-node/eth"
Expand All @@ -12,9 +16,6 @@ import (
"github.com/ethereum/go-ethereum/log"
"github.com/ethereum/go-ethereum/params"
"github.com/stretchr/testify/require"
"math/big"
"testing"
"time"
)

func setupFallbackClientTest(t Testing, sd *e2eutils.SetupData, log log.Logger, l1Url string) (*L1Miner, *L1Replica, *L1Replica, *L2Engine, *L2Sequencer, *sources.FallbackClient) {
Expand Down
15 changes: 10 additions & 5 deletions op-node/sources/fallback_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,13 @@ func (l *FallbackClient) switchCurrentRpc() {
}

func (l *FallbackClient) switchCurrentRpcLogic() error {
//Use defer to ensure that recoverIfFirstRpcHealth will always be executed regardless of the circumstances.
defer func() {
if !l.isInFallbackState {
l.isInFallbackState = true
l.recoverIfFirstRpcHealth()
}
}()
url := l.urlList[l.currentIndex]
newRpc, err := l.rpcInitFunc(url)
if err != nil {
Expand All @@ -179,10 +186,6 @@ func (l *FallbackClient) switchCurrentRpcLogic() error {
}
}
l.log.Info("switched current rpc to new url", "url", url)
if !l.isInFallbackState {
l.isInFallbackState = true
l.recoverIfFirstRpcHealth()
}
return nil
}

Expand Down Expand Up @@ -221,7 +224,9 @@ func (l *FallbackClient) recoverIfFirstRpcHealth() {
}
lastRpc := *l.currentRpc.Load()
l.currentRpc.Store(&l.firstRpc)
lastRpc.Close()
if lastRpc != l.firstRpc {
lastRpc.Close()
}
l.lastMinuteFail.Store(0)
l.currentIndex = 0
l.isInFallbackState = false
Expand Down
15 changes: 10 additions & 5 deletions op-service/client/fallback_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,13 @@ func (l *FallbackClient) switchCurrentClient() {
if l.lastMinuteFail.Load() <= l.fallbackThreshold {
return
}
//Use defer to ensure that recoverIfFirstRpcHealth will always be executed regardless of the circumstances.
defer func() {
if !l.isInFallbackState {
l.isInFallbackState = true
l.recoverIfFirstRpcHealth()
}
}()
l.currentIndex++
if l.currentIndex >= len(l.urlList) {
l.log.Error("the fallback client has tried all urls")
Expand All @@ -259,10 +266,6 @@ func (l *FallbackClient) switchCurrentClient() {
}
l.lastMinuteFail.Store(0)
l.log.Info("switched current rpc to new url", "url", url)
if !l.isInFallbackState {
l.isInFallbackState = true
l.recoverIfFirstRpcHealth()
}
}

func (l *FallbackClient) recoverIfFirstRpcHealth() {
Expand All @@ -287,7 +290,9 @@ func (l *FallbackClient) recoverIfFirstRpcHealth() {
}
lastClient := *l.currentClient.Load()
l.currentClient.Store(&l.firstClient)
lastClient.Close()
if lastClient != l.firstClient {
lastClient.Close()
}
l.lastMinuteFail.Store(0)
l.currentIndex = 0
l.isInFallbackState = false
Expand Down
Loading