You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "hemanth-19 (via GitHub)" <gi...@apache.org> on 2026/02/03 07:32:08 UTC

[PR] Add tpstatsjson nodetool subcommand for deterministic JSON thread-pool stats [cassandra]

hemanth-19 opened a new pull request, #4600:
URL: https://github.com/apache/cassandra/pull/4600

   ## Summary
   
   - Adds `nodetool tpstatsjson` — a new read-only subcommand that prints
     thread-pool and dropped-message statistics as a single, deterministic
     JSON object to stdout.
   - Output is **pretty-printed** (two-space indent, one value per line).
     This is deliberate: when a single counter ticks from `5` to `6` the
     diff is one line.  Compact single-line JSON would collapse the entire
     document into one line, turning every change into a full-document
     replacement and making diffs uninformative.  Users who need compact
     output for transport can pipe through `jq -c`.
   - Keys are alphabetically sorted at every nesting level (via Jackson
     `ORDER_MAP_ENTRIES_BY_KEYS`), making the output safe to `diff` in CI
     without post-processing.
   - Two-tier error handling: section-level and per-metric failures are
     each caught independently and recorded in a top-level `errors` object
     (present only when a failure actually occurs).  The `"N/A"` sentinel
     from unregistered MBeans is normalised to JSON `null` — data
     normalisation, not an error.
   
   ## Why a new command instead of fixing `tpstats -F json`
   
   `tpstats -F json` already emits JSON, but its key ordering is
   non-deterministic: the data flows through `TpStatsHolder.convert2Map()`
   and `StatsPrinter.JsonPrinter`, both of which use plain `HashMap`.
   Switching those to sorted maps would silently change the output that
   existing scripts and CI pipelines already consume and may parse or diff
   in an order-dependent way.  A dedicated command with its own
   `ObjectWriter` isolates the sorted-keys contract entirely — no existing
   code path is touched, no operator is surprised.
   
   ## Files changed
   
   | File | What |
   |------|------|
   | `src/java/.../TpStatsJson.java` | Command implementation (198 lines) |
   | `src/java/.../NodetoolCommand.java` | One-line registration in subcommands array |
   | `test/resources/nodetool/help/tpstatsjson` | Golden help file for `NodetoolHelpCommandsOutputTest` |
   | `test/unit/.../TpStatsJsonTest.java` | Six tests: validity, structure, key ordering, live-data visibility, N/A normalisation, errors serialisation |
   
   ## Static verification notes
   
   All API call sites and data contracts were cross-checked against the
   existing codebase before submission:
   
   - **NodeProbe signatures:** `getThreadPools()` returns
     `Multimap<String,String>` (`NodeProbe:2062`);
     `getThreadPoolMetric(path, pool, metric)` returns `Object`
     (`NodeProbe:1955`); `getDroppedMessages()` returns
     `Map<String,Integer>` (`NodeProbe:1672`).  Every call site in
     `TpStatsJson.execute()` matches these signatures exactly.
   - **Metric name parity with TpStatsHolder:** The five JMX attribute
     names used — `ActiveTasks`, `PendingTasks`, `CompletedTasks`,
     `CurrentlyBlockedTasks`, `TotalBlockedTasks` — are the identical set
     fetched by `TpStatsHolder:46–50`.  No new MBean attributes are
     introduced.
   - **Iteration pattern:** The
     `for (Map.Entry<String,String> tp : probe.getThreadPools().entries())`
     loop is the established pattern at `TpStatsHolder:43`.
   - **Output path:** `probe.output().out.println(…)` is the canonical
     write mechanism, used by 70+ existing commands.  `Output.out` is
     `public final PrintStream` (`Output:27`).
   - **Golden help file wrapping:** The NAME-section line break was
     verified by computing PicoCLI's 80-column word-wrap and
     cross-checking against the `failuredetector` reference file (a
     command with no custom options, structurally identical).  SYNOPSIS
     and OPTIONS sections are the shared global-JMX-option boilerplate.
     If the file is off by even one character `NodetoolHelpCommandsOutputTest`
     will surface the exact diff; regenerate via `NodetoolHelpGenerator`.
   
   ## Test plan
   
   Tests could not be executed locally (no `ant` / JDK runtime in the
   development environment).  The following three CI targets must pass:
   
   - [ ] `ant testsome -Dtest.name=org.apache.cassandra.tools.nodetool.TpStatsJsonTest`
         — all six new tests
   - [ ] `ant testsome -Dtest.name=org.apache.cassandra.tools.nodetool.TpStatsTest`
         — regression: existing tpstats behaviour unchanged
   - [ ] `ant testsome -Dtest.name=org.apache.cassandra.tools.nodetool.NodetoolHelpCommandsOutputTest`
         — golden help file validation (parameterised; auto-discovers tpstatsjson)
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)


-- 
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: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


Re: [PR] Add tpstatsjson nodetool subcommand for deterministic JSON thread-pool stats [cassandra]

Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on PR #4600:
URL: https://github.com/apache/cassandra/pull/4600#issuecomment-3856553289

   > tpstats -F json already emits JSON, but its key ordering is
   > non-deterministic: the data flows through TpStatsHolder.convert2Map()
   > and StatsPrinter.JsonPrinter, both of which use plain HashMap.
   > Switching those to sorted maps would silently change the output that
   > existing scripts and CI pipelines already consume and may parse or diff
   > in an order-dependent way.
   
   I do not understand the reasoning here. If tpstats -F prints non-deterministically, what harm is done when it starts to be ordered? If it is ordered, that is the same case as if it was not intentionally ordered and it was printed out ordered by pure accident. 
   


-- 
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: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org