Skip to content

tm-monitor: health need to update when add node or delete node (#2693)#2694

Merged
melekes merged 1 commit intotendermint:developfrom
WALL-E:tm-monitor-fix
Oct 25, 2018
Merged

tm-monitor: health need to update when add node or delete node (#2693)#2694
melekes merged 1 commit intotendermint:developfrom
WALL-E:tm-monitor-fix

Conversation

@WALL-E
Copy link
Contributor

@WALL-E WALL-E commented Oct 23, 2018

Fixed #2693

@melekes
Copy link
Contributor

melekes commented Oct 23, 2018

there's a data race

WARNING: DATA RACE
Read at 0x00c420122728 by goroutine 49:
  github.com/tendermint/tendermint/tools/tm-monitor/monitor.(*Network).RecalculateUptime()
      /go/src/github.com/tendermint/tendermint/tools/tm-monitor/monitor/network.go:107 +0x132
  github.com/tendermint/tendermint/tools/tm-monitor/monitor.(*Monitor).recalculateNetworkUptimeLoop()
      /go/src/github.com/tendermint/tendermint/tools/tm-monitor/monitor/monitor.go:204 +0x9b

Previous write at 0x00c420122728 by goroutine 47:
  github.com/tendermint/tendermint/tools/tm-monitor/monitor.(*Network).updateHealth()
      /go/src/github.com/tendermint/tendermint/tools/tm-monitor/monitor/network.go:163 +0x14e
  github.com/tendermint/tendermint/tools/tm-monitor/monitor.(*Network).NewNode()
      /go/src/github.com/tendermint/tendermint/tools/tm-monitor/monitor/network.go:145 +0xd2
  github.com/tendermint/tendermint/tools/tm-monitor/monitor.(*Monitor).Monitor()
      /go/src/github.com/tendermint/tendermint/tools/tm-monitor/monitor/monitor.go:96 +0x2a4
  github.com/tendermint/tendermint/tools/tm-monitor/monitor_test.TestMonitorUpdatesNumberOfValidators()
      /go/src/github.com/tendermint/tendermint/tools/tm-monitor/monitor/monitor_test.go:23 +0x87
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:777 +0x16d

Goroutine 49 (running) created at:
  github.com/tendermint/tendermint/tools/tm-monitor/monitor.(*Monitor).Start()
      /go/src/github.com/tendermint/tendermint/tools/tm-monitor/monitor/monitor.go:152 +0x68
  github.com/tendermint/tendermint/tools/tm-monitor/monitor_test.startMonitor()
      /go/src/github.com/tendermint/tendermint/tools/tm-monitor/monitor/monitor_test.go:54 +0xc7
  github.com/tendermint/tendermint/tools/tm-monitor/monitor_test.TestMonitorUpdatesNumberOfValidators()
      /go/src/github.com/tendermint/tendermint/tools/tm-monitor/monitor/monitor_test.go:19 +0x3c
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:777 +0x16d

Goroutine 47 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:824 +0x564
  testing.runTests.func1()
      /usr/local/go/src/testing/testing.go:1063 +0xa4
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:777 +0x16d
  testing.runTests()
      /usr/local/go/src/testing/testing.go:1061 +0x4e1
  testing.(*M).Run()
      /usr/local/go/src/testing/testing.go:978 +0x2cd
  main.main()
      _testmain.go:124 +0x324

@WALL-E
Copy link
Contributor Author

WALL-E commented Oct 24, 2018

fix DATA RACE

@codecov-io
Copy link

codecov-io commented Oct 25, 2018

Codecov Report

Merging #2694 into develop will decrease coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           develop   #2694      +/-   ##
==========================================
- Coverage    61.53%   61.5%   -0.04%     
==========================================
  Files          207     207              
  Lines        16935   16942       +7     
==========================================
- Hits         10421   10420       -1     
- Misses        5648    5654       +6     
- Partials       866     868       +2
Impacted Files Coverage Δ
tools/tm-monitor/monitor/network.go 89.24% <100%> (+0.74%) ⬆️
consensus/reactor.go 70.58% <0%> (-0.91%) ⬇️
consensus/state.go 75.93% <0%> (-0.24%) ⬇️
node/id.go 0% <0%> (ø) ⬆️
privval/tcp_server.go 81.42% <0%> (+2.85%) ⬆️

@melekes melekes merged commit bbf15b3 into tendermint:develop Oct 25, 2018
@WALL-E WALL-E deleted the tm-monitor-fix branch October 25, 2018 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ebuchman ebuchman Awaiting requested review from ebuchman

@xla xla Awaiting requested review from xla

1 more reviewer

@melekes melekes melekes approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@WALL-E @melekes @codecov-io