hubcio commented on issue #4707:
URL: https://github.com/apache/iggy/issues/4707#issuecomment-6667363594
thanks @mmmmxa, the batching looks useful for the 1M streams work. with
primaries spread evenly across 3 nodes, 40-byte Commit entries and roughly
32-byte reply entries every 2 s work out to ~12 MB/s per node-pair direction at
1M idle partitions, before frame and transport overhead. a few things to settle
before this lands:
1. the primary's quorum window needs a rule tying it to the idle interval.
R24 keeps it at `commit_heartbeat_timeout` (5 s), but R23 only applies the 4x
rule to the backup timeout. an idle interval of 10 s with a 40 s idle timeout
passes the stated rules while leaving whole quorum windows without a heartbeat.
even the defaults give only 2-3 heartbeat opportunities per window. requiring
at least 4 idle intervals per quorum window would apply R9's minimum margin to
idle heartbeats too. with a 5 s window and R23's 500 ms grid, the largest
allowed idle interval is 1 s.
shared idle frames also correlate failures across groups. losing the
replies needed for quorum can step down many primaries together.
`begin_view_probe` stops admission and heartbeats, but this does not
necessarily wait for the 12 s backup timeout: `handle_request_start_view` makes
backups vote for the next view when they receive a probe from their current
primary. please cover dropped and delayed Commit and CommitReply batches around
window boundaries, including R28's reply delay. keeping a longer idle interval
by separating serving and leadership deadlines would need its own safety and
recovery rules.
2. R20 treats an unchanged commit point as idle, but that does not mean
there are no writes in flight. a stalled group can still have pending prepares,
and the first write to an idle group does not make it busy until it commits.
should pending writes force the busy state before `commit_min` advances?
otherwise the longer timeout also applies while writes are waiting. this needs
an explicit rule and a test for an idle group receiving a write that cannot
commit.
3. keep the metadata group busy, always. new logins commit a `Register` op
in the metadata log, and creates, deletes and session expiry go through the
metadata primary. if that primary dies while idle, the next login or metadata
write inherits the 12 s failure-detection window, plus the view change. it is
one group, so the traffic saving is negligible.
4. one idle send phase per shard means big bursts at scale. at 1M on 3 nodes
with 8 shards each and balanced primaries, a shard leads ~42k groups. under
R1/R20 that is ~1.7 MB of Commit entries per peer in a single tick every 2 s.
with the default `replica_connections_per_peer = 1`, prepares queued behind
that frame share its TCP stream. R16/R17 also need a bound on entry processing
per pump turn. how about hashing groups into a few phases per interval,
batching each phase, and processing large batches in bounded chunks? phases
would spread the backup deadlines too. please measure pump latency and prepare
latency under load. also plz treat it as low prio. maybe this isn't as
impacting as im imagining.
5. this overlaps with my 1M streams work. i'm changing the tick to visit
only partitions with due work, which touches the `CommitMessage` timer and tick
clock. the latest design keeps metadata and partition clocks separate, so we
need to agree on clock ownership and advancement, metadata-only ticks,
heartbeat ownership, and merge order. if the shard schedule builds entries
directly from the groups it leads, the per-partition commit timer can go away,
but its check for an observed newer view must still trigger probing. the
session retirement change also changes the replica wire. let's coordinate the
revisions as these PRs land so incompatible layouts cannot share a handshake
identity.
6. the results table appears to exclude `CommitReply`. the master row has
773.3 total frames against 769.7 Commit frames per 500 ms, leaving no room for
one reply per accepted Commit.
[#4658](https://github.com/apache/iggy/pull/4658), merged October 31, added
those replies. that suggests either an older baseline or counters that omit
them. can you include the commit SHAs and separate Commit/CommitReply counters,
then rerun against current master once reply batching is in?
7. breaking the config with the rename is fine with me. R22 requires
matching configuration across nodes, but the spec does not enforce it. the
replica handshake already rejects a different link count. checking the four
effective timing values there would turn a mismatch into a clear error instead
of inconsistent failure detection.
8. R13 needs a release-mode size check. a debug assertion does not enforce
the 64 MiB frame limit, and the receiver closes the connection on an oversized
frame. split at the limit or enforce an explicit capacity bound; dropping the
whole batch would just turn the overflow into heartbeat loss. R6 matches the
existing tick-coalesced eager commits. dropping `commit_checksum` and
`checkpoint_op` from the Commit entry is fine too: the Commit handler does not
read them.
once the PR is up, ping me and i'll review it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
