On 10/20/20 1:16 PM, Ilya Maximets wrote:
> While sending snapshots backlog on raft connections could quickly
> grow over 4GB and this will overflow raft-backlog counter.
>
> Let's report it in kB instead. (Using kB and not KB to match with
> ru_maxrss counter reported by kernel)
>
> Fixes: 3423cd97f88f ("ovsdb: Add raft memory usage to memory report.")
> Signed-off-by: Ilya Maximets <[email protected]>
> ---
> ovsdb/raft.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/ovsdb/raft.c b/ovsdb/raft.c
> index 708b0624c..3411323aa 100644
> --- a/ovsdb/raft.c
> +++ b/ovsdb/raft.c
> @@ -1020,13 +1020,14 @@ void
> raft_get_memory_usage(const struct raft *raft, struct simap *usage)
> {
> struct raft_conn *conn;
> + uint64_t backlog = 0;
> int cnt = 0;
>
> LIST_FOR_EACH (conn, list_node, &raft->conns) {
> - simap_increase(usage, "raft-backlog",
> - jsonrpc_session_get_backlog(conn->js));
> + backlog += jsonrpc_session_get_backlog(conn->js);
> cnt++;
> }
> + simap_increase(usage, "raft-backlog-kB", backlog / 1000);
> simap_increase(usage, "raft-connections", cnt);
> }
>
>
Hi Ilya,
This change looks OK to me. However, since "raft-backlog" was already
displayed in v2.14.0, should we consider not breaking backwards compatibility
and reporting both "raft-backlog" (potentially overflowed) and
"raft-backlog-kB" at "ovs-appctl .. memory/show"?
On the other hand, ovn-k8s parses the output of "memory/show" but ignores
"raft-backlog" for now [1] so at least from that perspective it should be fine.
Is this considered a user visible change?
Regards,
Dumitru
[1]
https://github.com/ovn-org/ovn-kubernetes/blob/dd1289c3ff3543798786baac30c9e9ad51aa27a4/go-controller/pkg/metrics/ovn_db.go#L292
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev