Skip to content

Commit

Permalink
fix: "Lazy" Stringers to defer Sprintf and Hash until logs print (#8845)
Browse files Browse the repository at this point in the history
Co-authored-by: Joe Abbey <joe.abbey@gmail.com>
  • Loading branch information
tnasu and joeabbey committed Jul 13, 2023
1 parent d475dfa commit 69da06e
Show file tree
Hide file tree
Showing 13 changed files with 87 additions and 56 deletions.
8 changes: 4 additions & 4 deletions blockchain/v2/routine.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (rt *Routine) setMetrics(metrics *Metrics) {
}

func (rt *Routine) start() {
rt.logger.Info(fmt.Sprintf("%s: run", rt.name))
rt.logger.Info("routine start", "msg", log.NewLazySprintf("%s: run", rt.name))
running := atomic.CompareAndSwapUint32(rt.running, uint32(0), uint32(1))
if !running {
panic(fmt.Sprintf("%s is already running", rt.name))
Expand Down Expand Up @@ -98,7 +98,7 @@ func (rt *Routine) start() {
return
}
rt.metrics.EventsOut.With("routine", rt.name).Add(1)
rt.logger.Debug(fmt.Sprintf("%s: produced %T %+v", rt.name, oEvent, oEvent))
rt.logger.Debug("routine start", "msg", log.NewLazySprintf("%s: produced %T %+v", rt.name, oEvent, oEvent))

// Skip rTrySchedule and rProcessBlock events as they clutter the history
// due to their frequency.
Expand All @@ -118,7 +118,7 @@ func (rt *Routine) start() {

// XXX: look into returning OpError in the net package
func (rt *Routine) send(event Event) bool {
rt.logger.Debug(fmt.Sprintf("%s: received %T %+v", rt.name, event, event))
rt.logger.Debug("routine send", "msg", log.NewLazySprintf("%s: received %T %+v", rt.name, event, event))
if !rt.isRunning() {
return false
}
Expand Down Expand Up @@ -150,7 +150,7 @@ func (rt *Routine) stop() {
return
}

rt.logger.Info(fmt.Sprintf("%s: stop", rt.name))
rt.logger.Info("routine stop", "msg", log.NewLazySprintf("%s: stop", rt.name))
rt.queue.Dispose() // this should block until all queue items are free?
}

Expand Down
8 changes: 4 additions & 4 deletions consensus/reactor.go
Original file line number Diff line number Diff line change
Expand Up @@ -1276,12 +1276,12 @@ func (ps *PeerState) SetHasVote(vote *types.Vote) {
}

func (ps *PeerState) setHasVote(height int64, round int32, voteType tmproto.SignedMsgType, index int32) {
logger := ps.logger.With(
ps.logger.Debug("setHasVote",
"peerH/R",
fmt.Sprintf("%d/%d", ps.PRS.Height, ps.PRS.Round),
log.NewLazySprintf("%d/%d", ps.PRS.Height, ps.PRS.Round),
"H/R",
fmt.Sprintf("%d/%d", height, round))
logger.Debug("setHasVote", "type", voteType, "index", index)
log.NewLazySprintf("%d/%d", height, round),
"type", voteType, "index", index)

// NOTE: some may be nil BitArrays -> no side effects.
psVotes := ps.getVoteBitArray(height, round, voteType)
Expand Down
42 changes: 21 additions & 21 deletions consensus/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -1030,7 +1030,7 @@ func (cs *State) enterNewRound(height int64, round int32) {
if cs.Height != height || round < cs.Round || (cs.Round == round && cs.Step != cstypes.RoundStepNewHeight) {
logger.Debug(
"entering new round with invalid args",
"current", fmt.Sprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step),
"current", log.NewLazySprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step),
)
return
}
Expand All @@ -1040,7 +1040,7 @@ func (cs *State) enterNewRound(height int64, round int32) {
logger.Debug("need to set a buffer and log message here for sanity", "start_time", cs.StartTime, "now", now)
}

logger.Debug("entering new round", "current", fmt.Sprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step))
logger.Debug("entering new round", "current", log.NewLazySprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step))

// Select the current height and round Proposer
cs.Proposer = cs.Validators.SelectProposer(cs.state.LastProofHash, height, round)
Expand Down Expand Up @@ -1108,12 +1108,12 @@ func (cs *State) enterPropose(height int64, round int32) {
if cs.Height != height || round < cs.Round || (cs.Round == round && cstypes.RoundStepPropose <= cs.Step) {
logger.Debug(
"entering propose step with invalid args",
"current", fmt.Sprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step),
"current", log.NewLazySprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step),
)
return
}

logger.Debug("entering propose step", "current", fmt.Sprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step))
logger.Debug("entering propose step", "current", log.NewLazySprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step))

defer func() {
// Done enterPropose:
Expand Down Expand Up @@ -1286,7 +1286,7 @@ func (cs *State) enterPrevote(height int64, round int32) {
if cs.Height != height || round < cs.Round || (cs.Round == round && cstypes.RoundStepPrevote <= cs.Step) {
logger.Debug(
"entering prevote step with invalid args",
"current", fmt.Sprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step),
"current", log.NewLazySprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step),
)
return
}
Expand All @@ -1297,7 +1297,7 @@ func (cs *State) enterPrevote(height int64, round int32) {
cs.newStep()
}()

logger.Debug("entering prevote step", "current", fmt.Sprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step))
logger.Debug("entering prevote step", "current", log.NewLazySprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step))

// Sign and broadcast vote as necessary
cs.stepTimes.ToPrevoteStep()
Expand Down Expand Up @@ -1350,7 +1350,7 @@ func (cs *State) enterPrevoteWait(height int64, round int32) {
if cs.Height != height || round < cs.Round || (cs.Round == round && cstypes.RoundStepPrevoteWait <= cs.Step) {
logger.Debug(
"entering prevote wait step with invalid args",
"current", fmt.Sprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step),
"current", log.NewLazySprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step),
)
return
}
Expand All @@ -1362,7 +1362,7 @@ func (cs *State) enterPrevoteWait(height int64, round int32) {
))
}

logger.Debug("entering prevote wait step", "current", fmt.Sprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step))
logger.Debug("entering prevote wait step", "current", log.NewLazySprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step))

defer func() {
// Done enterPrevoteWait:
Expand All @@ -1386,12 +1386,12 @@ func (cs *State) enterPrecommit(height int64, round int32) {
if cs.Height != height || round < cs.Round || (cs.Round == round && cstypes.RoundStepPrecommit <= cs.Step) {
logger.Debug(
"entering precommit step with invalid args",
"current", fmt.Sprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step),
"current", log.NewLazySprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step),
)
return
}

logger.Debug("entering precommit step", "current", fmt.Sprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step))
logger.Debug("entering precommit step", "current", log.NewLazySprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step))

defer func() {
// Done enterPrecommit:
Expand Down Expand Up @@ -1512,7 +1512,7 @@ func (cs *State) enterPrecommitWait(height int64, round int32) {
logger.Debug(
"entering precommit wait step with invalid args",
"triggered_timeout", cs.TriggeredTimeoutPrecommit,
"current", fmt.Sprintf("%v/%v", cs.Height, cs.Round),
"current", log.NewLazySprintf("%v/%v", cs.Height, cs.Round),
)
return
}
Expand All @@ -1524,7 +1524,7 @@ func (cs *State) enterPrecommitWait(height int64, round int32) {
))
}

logger.Debug("entering precommit wait step", "current", fmt.Sprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step))
logger.Debug("entering precommit wait step", "current", log.NewLazySprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step))

defer func() {
// Done enterPrecommitWait:
Expand All @@ -1543,12 +1543,12 @@ func (cs *State) enterCommit(height int64, commitRound int32) {
if cs.Height != height || cstypes.RoundStepCommit <= cs.Step {
logger.Debug(
"entering commit step with invalid args",
"current", fmt.Sprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step),
"current", log.NewLazySprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step),
)
return
}

logger.Debug("entering commit step", "current", fmt.Sprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step))
logger.Debug("entering commit step", "current", log.NewLazySprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step))

defer func() {
// Done enterCommit:
Expand Down Expand Up @@ -1581,7 +1581,7 @@ func (cs *State) enterCommit(height int64, commitRound int32) {
if !cs.ProposalBlockParts.HasHeader(blockID.PartSetHeader) {
logger.Info(
"commit is for a block we do not know about; set ProposalBlock=nil",
"proposal", cs.ProposalBlock.Hash(),
"proposal", log.NewLazyBlockHash(cs.ProposalBlock),
"commit", blockID.Hash,
)

Expand Down Expand Up @@ -1618,7 +1618,7 @@ func (cs *State) tryFinalizeCommit(height int64) {
// TODO: ^^ wait, why does it matter that we're a validator?
logger.Debug(
"failed attempt to finalize commit; we do not have the commit block",
"proposal_block", cs.ProposalBlock.Hash(),
"proposal_block", log.NewLazyBlockHash(cs.ProposalBlock),
"commit_block", blockID.Hash,
)
return
Expand All @@ -1634,7 +1634,7 @@ func (cs *State) finalizeCommit(height int64) {
if cs.Height != height || cs.Step != cstypes.RoundStepCommit {
logger.Debug(
"entering finalize commit step",
"current", fmt.Sprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step),
"current", log.NewLazySprintf("%v/%v/%v", cs.Height, cs.Round, cs.Step),
)
return
}
Expand All @@ -1659,11 +1659,11 @@ func (cs *State) finalizeCommit(height int64) {

logger.Info(
"finalizing commit of block",
"hash", block.Hash(),
"hash", log.NewLazyBlockHash(block),
"root", block.AppHash,
"num_txs", len(block.Txs),
)
logger.Debug(fmt.Sprintf("%v", block))
logger.Debug("committed block", "block", log.NewLazySprintf("%v", block))

fail.Fail() // XXX

Expand Down Expand Up @@ -2018,7 +2018,7 @@ func (cs *State) handleCompleteProposal(blockHeight int64) {
cs.Logger.Debug(
"updating valid block to new proposal block",
"valid_round", cs.Round,
"valid_block_hash", cs.ProposalBlock.Hash(),
"valid_block_hash", log.NewLazyBlockHash(cs.ProposalBlock),
)

cs.ValidRound = cs.Round
Expand Down Expand Up @@ -2193,7 +2193,7 @@ func (cs *State) addVote(vote *types.Vote, peerID p2p.ID) (added bool, err error
} else {
cs.Logger.Debug(
"valid block we do not know about; set ProposalBlock=nil",
"proposal", cs.ProposalBlock.Hash(),
"proposal", log.NewLazyBlockHash(cs.ProposalBlock),
"block_id", blockID.Hash,
)

Expand Down
4 changes: 3 additions & 1 deletion libs/os/os.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"os"
"os/signal"
"syscall"

"github.com/Finschia/ostracon/libs/log"
)

type logger interface {
Expand All @@ -20,7 +22,7 @@ func TrapSignal(logger logger, cb func()) {
signal.Notify(c, os.Interrupt, syscall.SIGTERM)
go func() {
for sig := range c {
logger.Info(fmt.Sprintf("captured %v, exiting...", sig))
logger.Info("signal trapped", "msg", log.NewLazySprintf("captured %v, exiting...", sig))
if cb != nil {
cb()
}
Expand Down
30 changes: 25 additions & 5 deletions libs/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,11 @@ func (bs *BaseService) Start() error {
atomic.StoreUint32(&bs.started, 0)
return ErrAlreadyStopped
}
bs.Logger.Info(fmt.Sprintf("Starting %v service", bs.name), "impl", bs.impl.String())
bs.Logger.Info("service start",
"msg",
log.NewLazySprintf("Starting %v service", bs.name),
"impl",
bs.impl.String())
err := bs.impl.OnStart()
if err != nil {
// revert flag
Expand All @@ -145,7 +149,11 @@ func (bs *BaseService) Start() error {
}
return nil
}
bs.Logger.Debug(fmt.Sprintf("Not starting %v service -- already started", bs.name), "impl", bs.impl)
bs.Logger.Debug("service start",
"msg",
log.NewLazySprintf("Not starting %v service -- already started", bs.name),
"impl",
bs.impl)
return ErrAlreadyStarted
}

Expand All @@ -165,12 +173,20 @@ func (bs *BaseService) Stop() error {
atomic.StoreUint32(&bs.stopped, 0)
return ErrNotStarted
}
bs.Logger.Info(fmt.Sprintf("Stopping %v service", bs.name), "impl", bs.impl)
bs.Logger.Info("service stop",
"msg",
log.NewLazySprintf("Stopping %v service", bs.name),
"impl",
bs.impl)
bs.impl.OnStop()
close(bs.quit)
return nil
}
bs.Logger.Debug(fmt.Sprintf("Stopping %v service (already stopped)", bs.name), "impl", bs.impl)
bs.Logger.Debug("service stop",
"msg",
log.NewLazySprintf("Stopping %v service (already stopped)", bs.name),
"impl",
bs.impl)
return ErrAlreadyStopped
}

Expand All @@ -183,7 +199,11 @@ func (bs *BaseService) OnStop() {}
// will be returned if the service is running.
func (bs *BaseService) Reset() error {
if !atomic.CompareAndSwapUint32(&bs.stopped, 1, 0) {
bs.Logger.Debug(fmt.Sprintf("Can't reset %v service. Not stopped", bs.name), "impl", bs.impl)
bs.Logger.Debug("service reset",
"msg",
log.NewLazySprintf("Can't reset %v service. Not stopped", bs.name),
"impl",
bs.impl)
return fmt.Errorf("can't reset running %s", bs.name)
}

Expand Down
6 changes: 3 additions & 3 deletions p2p/conn/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ func (c *MConnection) Send(chID byte, msgBytes []byte) bool {
return false
}

c.Logger.Debug("Send", "channel", chID, "conn", c, "msgBytes", fmt.Sprintf("%X", msgBytes))
c.Logger.Debug("Send", "channel", chID, "conn", c, "msgBytes", log.NewLazySprintf("%X", msgBytes))

// Send message to channel.
channel, ok := c.channelsIdx[chID]
Expand All @@ -376,7 +376,7 @@ func (c *MConnection) Send(chID byte, msgBytes []byte) bool {
default:
}
} else {
c.Logger.Debug("Send failed", "channel", chID, "conn", c, "msgBytes", fmt.Sprintf("%X", msgBytes))
c.Logger.Debug("Send failed", "channel", chID, "conn", c, "msgBytes", log.NewLazySprintf("%X", msgBytes))
}
return success
}
Expand All @@ -388,7 +388,7 @@ func (c *MConnection) TrySend(chID byte, msgBytes []byte) bool {
return false
}

c.Logger.Debug("TrySend", "channel", chID, "conn", c, "msgBytes", fmt.Sprintf("%X", msgBytes))
c.Logger.Debug("TrySend", "channel", chID, "conn", c, "msgBytes", log.NewLazySprintf("%X", msgBytes))

// Send message to channel.
channel, ok := c.channelsIdx[chID]
Expand Down
3 changes: 2 additions & 1 deletion p2p/pex/addrbook.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/minio/highwayhash"

"github.com/Finschia/ostracon/crypto"
"github.com/Finschia/ostracon/libs/log"
tmmath "github.com/Finschia/ostracon/libs/math"
tmrand "github.com/Finschia/ostracon/libs/rand"
"github.com/Finschia/ostracon/libs/service"
Expand Down Expand Up @@ -740,7 +741,7 @@ func (a *addrBook) expireNew(bucketIdx int) {
for addrStr, ka := range a.bucketsNew[bucketIdx] {
// If an entry is bad, throw it away
if ka.isBad() {
a.Logger.Info(fmt.Sprintf("expiring bad address %v", addrStr))
a.Logger.Info("expire new", "msg", log.NewLazySprintf("expiring bad address %v", addrStr))
a.removeFromBucket(ka, bucketTypeNew, bucketIdx)
return
}
Expand Down
3 changes: 2 additions & 1 deletion p2p/switch.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/Finschia/ostracon/config"
"github.com/Finschia/ostracon/libs/cmap"
"github.com/Finschia/ostracon/libs/log"
"github.com/Finschia/ostracon/libs/rand"
"github.com/Finschia/ostracon/libs/service"
"github.com/Finschia/ostracon/p2p/conn"
Expand Down Expand Up @@ -262,7 +263,7 @@ func (sw *Switch) OnStop() {
//
// NOTE: Broadcast uses goroutines, so order of broadcast may not be preserved.
func (sw *Switch) Broadcast(chID byte, msgBytes []byte) chan bool {
sw.Logger.Debug("Broadcast", "channel", chID, "msgBytes", fmt.Sprintf("%X", msgBytes))
sw.Logger.Debug("Broadcast", "channel", chID, "msgBytes", log.NewLazySprintf("%X", msgBytes))

peers := sw.peers.List()
var wg sync.WaitGroup
Expand Down
Loading

0 comments on commit 69da06e

Please sign in to comment.