log.proto: add LeaderInfo; add node_group_leader_info to WriteLogResponse

[MrGuin]

Related PR:
eloqdata/tx_service#134
https://github.com/eloqdata/eloq_log_service/pull/18

Summary by CodeRabbit

• New Features
  • Write responses now include current leader details for each node group (leader term, IP, and port), improving cluster visibility and enabling clients to route requests to the right node faster.
  • Enhances observability of leadership changes across node groups, aiding diagnostics and reducing latency during leader transitions.

[coderabbitai]

Walkthrough

Adds a new protobuf message LeaderInfo and augments WriteLogResponse with a node_group_leader_info map field to return leader details per node group. No other messages are modified.

Changes

Cohort / File(s)	Summary
Protobuf schema updates log.proto	Added message LeaderInfo { int64 term; bytes ip; uint32 port; }. Extended WriteLogResponse with field 3: node_group_leader_info as map<uint32, LeaderInfo>. No other schema changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant S as LogService
  participant CM as ClusterMeta

  rect rgba(227,242,253,0.4)
  note over C,S: WriteLog flow with leader info included
  C->>S: WriteLog(request)
  S->>S: Append entry
  S->>CM: Fetch leaders per node group
  CM-->>S: node_group_leader_info (map<groupId, LeaderInfo>)
  S-->>C: WriteLogResponse{ status, node_group_leader_info }
  end

  note over C: Client may cache/update leader mapping per group

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nibble bytes where leaders glow,
A map of groups now helps me know;
With terms and ports, and IP too,
I hop through logs to guide the crew.
New fields bloom bright—proto delight! 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly and accurately describes the primary change in the changeset: adding a new LeaderInfo message and adding node_group_leader_info to WriteLogResponse in log.proto, which matches the PR summary and file diffs; it is concise, specific, and free of noise so a teammate scanning history will understand the main intent.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch ddl_log_ng_leader

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

log.proto (2)

265-269: Prefer string ip (IPv4/IPv6) and include a stable node_id to avoid future breaking change.

Across this proto, IPs are modeled as string (e.g., ReplayLogRequest.source_ip, AddPeerRequest.ip, GetLogGroupConfigResponse.ip). Using bytes ip here is inconsistent, complicates cross‑lang handling (endian/length/IPv4 vs IPv6), and will be a breaking change if we switch later. Also, leaders are already identified via node_id elsewhere (LogLeaderUpdateRequest.node_id, NodeConfig.node_id); relying only on ip/port couples clients to network addressing and makes readdressing harder.

Recommend aligning now:

 message LeaderInfo {
   int64 term = 1;
-  bytes ip = 2;
+  // String literal to support IPv4 and IPv6; align with existing messages.
+  string ip = 2;
   uint32 port = 3;
+  // Stable identifier; helps when IPs change.
+  uint32 node_id = 4;
 }

Optional: add a short comment that port must be in [1, 65535] and that ip should be a literal (not hostname).

---

274-276: Scope and payload: clarify when node_group_leader_info is populated; consider sending only on redirects.

Returning leader info for all node groups on every write may add non‑trivial payload and churn. If the primary consumer is retry logic upon NotLeader or redirects, consider populating this map only in those cases, or gate it behind a request flag for clients that want it. At minimum, document the semantics (always present vs best‑effort vs only on redirect), and ensure clients treat it as optional (map may be empty).

If you agree to restrict emission to redirects, add a brief comment:

   // latest leader info of all node groups
-  map<uint32, LeaderInfo> node_group_leader_info = 3;
+  // Populated on redirects or when leadership may have changed; may be empty otherwise.
+  map<uint32, LeaderInfo> node_group_leader_info = 3;

Also confirm the map key is the same “cc node group id” used in WriteLogRequest.node_terms for consistency.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a5bde4c and 7feee7e.

📒 Files selected for processing (1)

• log.proto (1 hunks)