[#18983] DocDB: Sort yb-admin list_all_tablet_servers by status then host#32476
[#18983] DocDB: Sort yb-admin list_all_tablet_servers by status then host#32476ag26jan wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces sorting functionality for the tablet server list in yb-admin, ensuring alive servers are listed before dead ones, followed by host and UUID sorting. It also includes a new test suite to verify this behavior. The feedback suggests optimizing the sorting implementation in SortListTabletServerEntries to sort the RepeatedPtrField in-place directly, avoiding unnecessary auxiliary allocations and deep copies of protobuf messages.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces sorting for the tablet server list in yb-admin, ordering alive servers before dead ones, followed by host/port, and using the UUID as a tie-breaker. The feedback suggests optimizing the comparator to avoid dynamic memory allocations by directly comparing protobuf fields, removing an unused header as a result, and expanding the unit tests to cover the UUID tie-breaker logic when hostnames are identical.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements sorting for the tablet server list in yb-admin so that alive servers are displayed before dead ones, with secondary sorting by host, port, and UUID. A unit test was also added to verify this behavior. The review feedback recommends replacing the global kEmptyHost string with std::string_view in the comparison helper to prevent potential static initialization/destruction order issues and avoid copy overhead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…us then host Sort tablet server list output alive-first, then by RPC host, port, and UUID tie-break. Sort in place on RepeatedPtrField and compare protobuf fields directly in the comparator. Add a unit test for sort ordering. Co-authored-by: Cursor <cursoragent@cursor.com>
ag26jan
force-pushed
the
db-8417-yb-admin-list-tservers-sort
branch
from
d1d95fc to
83ef229
Compare
1 participant
Summary
Sort
yb-admin list_all_tablet_serversoutput for easier operator scanning at scale.SortListTabletServerEntries()and covered by a unit testFixes #18983
Testing Done:
./yb_build.sh release --cxx-test yb-admin_client-test --gtest_filter ListTabletServerSortTest.AliveBeforeDeadThenHost --sj --skip-pg-parquet --no-odyssey --no-ybcbuild/release-clang21-dynamic-arm64-ninja/bin/yb-admin list_all_tablet_serversshows alive hosts first, then dead hosts, each group sorted by hostnameAfter fix:
Before Fix:
For all alive nodes normal behavior:
But after fix:
What you're seeing
All 15 rows are ALIVE, so the first key (alive before dead) does not change anything. Ordering is purely by hostname string compare (alphabetical/lexicographic).
node1...node1node10–node15...node1, the next char is0–5, which sorts before...node2node2–node9...node2comes after...node15in string orderSo this order:
is exactly what string sort produces.
Why
node10is beforenode2String compare is character-by-character:
That is the classic
node2vsnode10lexicographic ordering.Bottom line: The sort matches the agreed behavior. The surprising order is from lexicographic hostnames, not a broken comparator.
In real world the sorting will look more simple and human readable because nodes generally follows name > AZ.