-
Notifications
You must be signed in to change notification settings - Fork 198
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.
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
Bug fix panic tps benchmark #2279
Conversation
core/statistics/tpsBenchmark.go
Outdated
@@ -179,8 +179,13 @@ func (s *TpsBenchmark) NrOfShards() uint32 { | |||
// ShardStatistics returns the current statistical state for a given shard | |||
func (s *TpsBenchmark) ShardStatistics() map[uint32]ShardStatistic { | |||
s.mut.RLock() | |||
defer s.mut.RUnlock() | |||
return s.shardStatistics | |||
copyMapShardStatistics := make(map[uint32]ShardStatistic) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine, I see that updateStatistics
is protected by the same lock.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename to shardStatisticsMapCopy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
core/statistics/tpsBenchmark.go
Outdated
@@ -179,8 +179,13 @@ func (s *TpsBenchmark) NrOfShards() uint32 { | |||
// ShardStatistics returns the current statistical state for a given shard | |||
func (s *TpsBenchmark) ShardStatistics() map[uint32]ShardStatistic { | |||
s.mut.RLock() | |||
defer s.mut.RUnlock() | |||
return s.shardStatistics | |||
copyMapShardStatistics := make(map[uint32]ShardStatistic) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename to shardStatisticsMapCopy
func TestTpsBenchmark_ShardStatisticConcurrentAccess(t *testing.T) { | ||
t.Parallel() | ||
|
||
tpsBenchmark, _ := statistics.NewTPSBenchmark(12, 4) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use recover() as you should check that no panic has encountered during the test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
System tests passed
Fix concurrent access on map that store shard statistics information for TPS Benchmark.
When method ShardStatistics() is called will return a copy of map that contains shard statistics.