Skip to content

Cassandra QAT update optimization#25

Open
ssherman8 wants to merge 20 commits intointel:mainfrom
ssherman8:cassandra_qat_update
Open

Cassandra QAT update optimization#25
ssherman8 wants to merge 20 commits intointel:mainfrom
ssherman8:cassandra_qat_update

Conversation

@ssherman8
Copy link
Copy Markdown

The purpose of this PR is show benefits of using Cassandra with QAT. It has been reviewed by both the Java team and the QAT team. The PCR for the performance claim was approved in PCR-1423

@harp-intel
Copy link
Copy Markdown
Contributor

Claude Code review:

Summary

This PR adds a new guide (software/cassandra/QAT/README.md) for using Cassandra with Intel QAT hardware-accelerated compression via the zlib-accel
library. It also adds a link in the root README.md. +168 lines, 2 files changed.

Existing Review Comments (by rsiyer-intel)

There are 4 unresolved comments that should be addressed:

  1. L89 — Typo: "package manger" → "package manager"
  2. L148 — "6 independent Cassandra servers and servers" — likely should be "clients and servers"
  3. L120 — Missing link to the zlib-accel repo in the build section + should add an explicit git clone to /opt/zlib-accel since that path is referenced in
    the LD_PRELOAD command
  4. L163 — Typo in references: "zib-accel" → "zlib-accel"

Additional Issues I Found

Typos / grammar:

  • L30: "be reading to use" → "be ready to use"
  • L48: "The output of the command with be" → "will be"
  • L89: "that with the distribution's" → "them with the distribution's"
  • L108: broken markdown link [optimization-zone] (https://...) — remove the space before (
  • L153: broken markdown link [enhancement proposal] (https://...) — same space issue
  • L40: duplicate wget commands — qat_4xxx.bin is downloaded twice instead of qat_4xxx_mmp.bin, and same pattern for qat_402xx and qat_420xx (the _mmp.bin
    variants are missing)

Content issues:

  • L12: ToC anchor #Details has capital D but the heading is ## Details — may not link correctly depending on renderer (GitHub lowercases anchors, so
    #details would be correct)
  • L119: The cmake command has -CMAKE_BUILD_TYPE=Release — should this be -DCMAKE_BUILD_TYPE=Release?
  • L148: No mention of the NoSQLBench workload parameters (e.g., thread count, duration) — would help reproducibility
  • The firmware download section downloads both .bin and _mmp.bin variants but the wget commands only fetch .bin files (duplicated) — the _mmp.bin files
    are never actually downloaded

Missing context:

  • No link or instructions for where to get the zlib-accel source code before the build steps (aligns with rsiyer-intel's comment)
  • The cassandra.yaml compression settings aren't shown — readers need to know which sstable_compression class to use with zlib

Overall the guide covers the right topics but needs a pass for typos, broken links, and the duplicate/missing firmware download commands before merging.

Copy link
Copy Markdown
Collaborator

@rsiyer-intel rsiyer-intel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Sri. Left few comments and suggestions.

ssherman8 and others added 5 commits March 27, 2026 18:19
Typo

Co-authored-by: rsiyer-intel <rukmini.subramanian.iyer@intel.com>
Typo

Co-authored-by: rsiyer-intel <rukmini.subramanian.iyer@intel.com>
Typo

Co-authored-by: rsiyer-intel <rukmini.subramanian.iyer@intel.com>
Co-authored-by: rsiyer-intel <rukmini.subramanian.iyer@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants