chore: Add stats print for slot migrations #4456
Conversation
| @@ -331,7 +350,7 @@ void RestoreStreamer::WriteEntry(string_view key, const PrimeValue& pk, const Pr | |||
| ThrottleIfNeeded(); | |||
| }, | |||
| ServerState::tlocal()->serialization_max_chunk_size); | |||
| serializer.SerializeEntry(key, pk, pv, expire_ms); | |||
| stats_.commands += serializer.SerializeEntry(key, pk, pv, expire_ms); | |||
There was a problem hiding this comment.
This actually counting the number of flush to sync that we have right (number of times you call commit)? not the number of commands generated from each entry.
There was a problem hiding this comment.
Excluding timeout and stickiness they should be the same
There was a problem hiding this comment.
Ok I see now so this is not actually the number of times we actually flush to sync but the time we write to the pending_buffer.
src/server/journal/streamer.cc
Outdated
| @@ -310,6 +327,8 @@ void RestoreStreamer::OnDbChange(DbIndex db_index, const DbSlice::ChangeReq& req | |||
| std::lock_guard guard(big_value_mu_); | |||
| DCHECK_EQ(db_index, 0) << "Restore migration only allowed in cluster mode in db0"; | |||
|
|
|||
| stats_.buckets_on_db_update++; | |||
There was a problem hiding this comment.
I think that it will be better to count actually how many buckets where serialized on ondbchange i,e Write bucket will return true if the bucket was serialized false if skipped
This way if we have a very week test that will send few commands but we almost covered the traveres we will skip the write bucket in this flow and would not be able to detect such week tests
There was a problem hiding this comment.
To clarify, you want this in order to handle potential CVCUponInsert() handling multiple buckets? If so we can increase it there, if not, I think I did not understand your comment
There was a problem hiding this comment.
I want to understand that buckets where actually serialized from this flow and not only the command was called. Because if it is called after the bucket is serialized than we dont do anything (skip write bucket)
There was a problem hiding this comment.
oh gotcha, sure thing!
src/server/journal/cmd_serializer.cc
Outdated
| // We send RESTORE commands for small objects, or objects we don't support breaking. | ||
| bool use_restore_serialization = true; | ||
| size_t commits = 1; |
There was a problem hiding this comment.
how about rename to flush
src/server/journal/cmd_serializer.cc
Outdated
| // We send RESTORE commands for small objects, or objects we don't support breaking. | ||
| bool use_restore_serialization = true; | ||
| size_t flushes = 1; |
There was a problem hiding this comment.
After I dive into the code again related to your above comment of flush is the same as the number of commands. But this is not true. We actually accumulating the commands. You dont know if the data was actually written to socket
There was a problem hiding this comment.
renamed to commands
|
@adiholden please re-approve, I had to tweak asserted number of keys because |
Fixes #4415