Add network-bytes-out metric support for CLUSTER SLOT-STATS command (#20)

[kyle-yh-kim]

Update, July 24th 2024

This PR has been cherry-picked into an existing network-bytes-in PR; #720
Both network-bytes-in and out will be tracked under a single PR. I will now close this PR.

---

The metric tracks network egress bytes under per-slot context, by hooking onto COB buffer mutations.

The metric can be viewed by calling the CLUSTER SLOT-STATS command, with sample response attached below;

127.0.0.1:6379> cluster slot-stats slotsrange 0 0
1) 1) (integer) 0
   2) 1) "key-count"
      2) (integer) 1
      3) "network-bytes-out"
      4) (integer) 175

[kyle-yh-kim]

Currently, the PR aggregates network-bytes-out from three different sources;

1. User client's outgoing RESP response, as a result of command invocation.
2. Replica client's outgoing replication stream.
3. Outgoing message due to sharded pubsub publishment.

If this feels too confusing, there are two alternatives;

1. Drop the replication stream and sharded pubsub accounting altogether, and strictly track only user client’s and valkey-server’s interaction.
2. Split the network-bytes-out metric into three sub-components;

   127.0.0.1:6379> cluster slot-stats slotsrange 12539 12539
   1) 1) (integer) 12539
     2) 1) "key-count"
        2) (integer) 1
        3) "network-bytes-out-user"
        4) (integer) 10
        5) "network-bytes-out-replication"
        6) (integer) 20
        7) "network-bytes-out-sharded-pubsub"
        8) (integer) 30
   

Personally, I would be opposed to dropping the replication stream and sharded pubsub requirements altogether, as the inclusion of all three components better represents the actual egress bytes. Curious to hear the core team’s thoughts on this.

[hpatro]

Suggested change

	void clusterSlotStatsAddNetworkBytesOutForReplication(int len) {
	void clusterSlotStatsAddNetworkBytesOutForReplication(size_t len) {

[hpatro]

Not clearly following this trick and seems dangerous. If we do an early exit on the following statement, we don't restore the value in c->slot. Is that intentional ?

[kyle-yh-kim]

Good catch, this was not done intentionally. The command should stay idempotent, irregardless of its early return condition.

Update has been reflected on the other PR, which replaces this one; #720

[hpatro]

This could also go into afterCommand ?

[kyle-yh-kim]

This PR has been cherry-picked into an existing network-bytes-in PR; #720
Both network-bytes-in and out will be tracked under a single PR. I will now close this PR.