On 09/06/22 11:19, Eric Blake wrote:
On Mon, Sep 05, 2022 at 02:10:36PM +0200, Laszlo Ersek wrote:
> On 09/03/22 00:14, Eric Blake wrote:
>> This proves that the stat counters increment as desired, as well as
>> proving that our RUInt32 generator type works.
>
> How is this related to RUint32?
Typo in the commit message; this should be RUInt64. Oh well, the
commit is already in.
>
>>
>> This commit is also a showcase of whether I can do 64-bit math in
>> various languages (C's terseness in 'a == b + (c > d)' is annoying
to
>> replicate in languages that don't like playing fast and loose with
>> types). :)
>
> I don't understand; please elaborate.
Sure. In C, uint64_t == uint64_t + bool is legal; making for a nice
terse expression.
I interpreted your statement about 64-bit math and the formula
a == b + (c > d)
as if the formula were some means to implement *64-bit-safe* math (addition, overflow etc
something like that). I understood how the expression worked at the lowest level, but
didn't understand what it was good for -- what it expressed.
The more concrete form is
cr3 == cr2 + (br3 > br2)
which seems to stand for: "chunks received #3" should equal "chunks
received #2, plus one chunk, if we received additional bytes (in br3) since setting
br2".
Basically the section
+bs2 = h.stats_bytes_sent()
+cs2 = h.stats_chunks_sent()
+br2 = h.stats_bytes_received()
+cr2 = h.stats_chunks_received()
+
+[...]
+
+# Stats are still readable after the connection closes; we don't know if
+# the server sent reply bytes to our NBD_CMD_DISC, so don't insist on it.
+h.shutdown()
+
+bs3 = h.stats_bytes_sent()
+cs3 = h.stats_chunks_sent()
+br3 = h.stats_bytes_received()
+cr3 = h.stats_chunks_received()
+
+assert bs3 > bs2
+assert cs3 == cs2 + 1
+assert br3 >= br2
+assert cr3 == cr2 + (br3 > br2)
was a bit too hard for me to parse. I didn't notice the role of h.shutdown() before.
Acked-by: Laszlo Ersek <lersek(a)redhat.com>
>> +++ b/python/t/620-stats.py
>> +# Stats are still readable after the connection closes; we don't know if
>> +# the server sent reply bytes to our NBD_CMD_DISC, so don't insist on it.
>> +h.shutdown()
>> +
>> +bs3 = h.stats_bytes_sent()
>> +cs3 = h.stats_chunks_sent()
>> +br3 = h.stats_bytes_received()
>> +cr3 = h.stats_chunks_received()
>> +
>> +assert bs3 > bs2
>> +assert cs3 == cs2 + 1
>> +assert br3 >= br2
>> +assert cr3 == cr2 + (br3 > br2)
And in python.
>> +++ b/ocaml/tests/test_620_stats.ml
>> + (* Stats are still readable after the connection closes; we don't know if
>> + * the server sent reply bytes to our NBD_CMD_DISC, so don't insist on
it.
>> + *)
>> + NBD.shutdown nbd;
>> +
>> + let bs3 = NBD.stats_bytes_sent nbd in
>> + let cs3 = NBD.stats_chunks_sent nbd in
>> + let br3 = NBD.stats_bytes_received nbd in
>> + let cr3 = NBD.stats_chunks_received nbd in
>> + let fudge = if cr2 = cr3 then 0L else 1L in
>> + assert (bs3 > bs2);
>> + assert (cs3 = (Int64.succ cs2));
>> + assert (br3 >= br2);
>> + assert (cr3 = (Int64.add cr2 fudge))
Not so in OCaml, where + isn't even polymorphic to int64, so I have to
break out manual calls to Int64.XXX methods. Here, I went with a
helper variable 'fudge'.
>> +++ b/golang/libnbd_620_stats.go
>> + /* Stats are still readable after the connection closes; we don't know if
>> + * the server sent reply bytes to our NBD_CMD_DISC, so don't insist on it.
>> + */
>> + err = h.Shutdown(nil)
>> + if err != nil {
>> + t.Fatalf("%s", err)
>> + }
>> +
>> + bs3, err := h.StatsBytesSent()
>> + if err != nil {
>> + t.Fatalf("%s", err)
>> + }
>> + cs3, err := h.StatsChunksSent()
>> + if err != nil {
>> + t.Fatalf("%s", err)
>> + }
>> + br3, err := h.StatsBytesReceived()
>> + if err != nil {
>> + t.Fatalf("%s", err)
>> + }
>> + cr3, err := h.StatsChunksReceived()
>> + if err != nil {
>> + t.Fatalf("%s", err)
>> + }
>> + slop := uint64(1)
>> + if br2 == br3 {
>> + slop = uint64(0)
>> + }
>> +
>> + if bs3 <= bs2 {
>> + t.Fatalf("unexpected value for bs3")
>> + }
>> + if cs3 != cs2 + 1 {
>> + t.Fatalf("unexpected value for cs3")
>> + }
>> + if br3 < br2 {
>> + t.Fatalf("unexpected value for br3")
>> + }
>> + if cr3 != cr2 + slop {
>> + t.Fatalf("unexpected value for cr3")
>> + }
Nor in Go, where you can't even do uint64(bool), but HAVE to use an
'if' statement to populate 'slop' with the correct value. Hmm, maybe
I should have used the same variable name between OCaml and Go,
instead of 'slop'/'fudge'; oh well. (And Go is painfully verbose in
the amount of boilerplate formatting it requires, compared to the
other languages; although I will be the first to admit that I'm
probably not an idiomatic Go coder)