[INDY-1582] add median for calculating average throughput and latency for backups #871
Conversation
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
plenum/server/monitor.py
Outdated
| "{}'s master's avg request latency is {} and backup's " | ||
| "avg request latency is {}".format(self, avgLatM, avgLatB)) | ||
| return True | ||
| if latencies: |
There was a problem hiding this comment.
if not latencies:
continue
plenum/server/monitor.py
Outdated
| high_avg_lat = self.latency_avg_strategy_cls.get_avg(latencies) | ||
| avg_master_lat = avgLatM[cid] | ||
| if avg_master_lat - high_avg_lat < self.Omega: | ||
| return False |
There was a problem hiding this comment.
continue instead of return?
plenum/server/monitor.py
Outdated
| avg_master_lat = avgLatM[cid] | ||
| if avg_master_lat - high_avg_lat < self.Omega: | ||
| return False | ||
| else: |
There was a problem hiding this comment.
redundant else sine continue/return is above
plenum/server/monitor.py
Outdated
| return False | ||
| else: | ||
| d = avg_master_lat - high_avg_lat | ||
| logger.info("{}{} found difference between master's and " |
There was a problem hiding this comment.
Why two logs (info and warning) here? Please combine to one (info level)
|
|
||
| # If latency of the master for any client is greater than that of | ||
| # backups by more than the threshold `Omega`, then a view change | ||
| # needs to happen | ||
| for cid, lat in avgLatB.items(): | ||
| for cid, latencies in avgLatB.items(): |
There was a problem hiding this comment.
Do we really need to check it for each client? Maybe average is enough?
When do we clear latency for each client? Is it a long operation to loop over all cids?
There was a problem hiding this comment.
Will be changed in https://jira.hyperledger.org/browse/INDY-1599
| for lat_item in [self.monitor.getLatencies(instId) for instId in self.instances.backupIds]: | ||
| for cid, lat in lat_item.items(): | ||
| backup_latencies.setdefault(cid, []).append(lat) | ||
| backup_latencies = [mean(lat) for cid, lat in backup_latencies.items()] |
There was a problem hiding this comment.
We show mean value here (for all clients), while monitor checks it for each client independently.
| @@ -38,11 +40,12 @@ def testReqLatencyThreshold(looper, txnPoolNodeSet, requests): | |||
| assert node.monitor.masterReqLatenciesTest[key] <= node.monitor.Lambda | |||
|
|
|||
|
|
|||
| @pytest.mark.skip(reason="Logic is checked in unit tests") | |||
There was a problem hiding this comment.
Remove the test then?
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
No description provided.