You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "He-Pin (via GitHub)" <gi...@apache.org> on 2026/05/31 18:06:51 UTC

[PR] feat: replace CountMinSketch with FastFrequencySketch in Artery compression [pekko]

He-Pin opened a new pull request, #3023:
URL: https://github.com/apache/pekko/pull/3023

   ### Motivation
   
   The Artery compression system uses a `CountMinSketch` (128KB per connection, 16×1024×8 bytes) for heavy hitter detection in actor ref and class manifest compression tables. Pekko already has `FastFrequencySketch` in `pekko-actor` (used for cluster sharding entity passivation) which is:
   
   - **~32x more memory efficient** (~4KB vs ~128KB per inbound connection)
   - **Includes TinyLFU reset** — periodic halving of all counters provides natural aging, enabling the heavy hitters table to adapt to changing traffic patterns over time
   - **Already battle-tested** — used in production cluster sharding passivation strategies
   
   This addresses [akka/akka-core#31093](https://github.com/akka/akka-core/issues/31093).
   
   ### Modification
   
   - **`InboundCompressions.scala`**: Replace `CountMinSketch` with `FastFrequencySketch[T]` in `InboundCompression`. The `increment` method now calls `frequencySketch.increment(value)` per occurrence and uses `frequencySketch.frequency(value)` as the estimated weight for heavy hitter detection.
   - **`TopHeavyHitters.scala`**: 
     - `update()` now always allows weight updates for existing heavy hitters (previously only allowed increases). This is necessary because `FrequencySketch`'s periodic reset halves all counters.
     - `updateExistingHeavyHitter()` supports both weight increase (push down) and decrease (bubble up) in the heap.
     - New `fixHeapUp()` method for bidirectional heap property restoration.
     - `isHeavy()` check now only gates insertion of **new** entries.
   - **`CountMinSketch.java`**: Removed — was `INTERNAL API` with no external users.
   - **`CountMinSketchBenchmark.scala`**: Updated to benchmark `FastFrequencySketch` instead.
   - **Legal files**: Removed CountMinSketch/stream-lib attribution (no longer bundled).
   
   ### Result
   
   Artery compression uses ~4KB instead of ~128KB per inbound connection. Heavy hitters now adapt to changing traffic patterns via TinyLFU aging rather than tracking monotonically increasing absolute counts.
   
   ### Tests
   
   - `remote / testOnly ...HeavyHittersSpec`: 13/13 passed (includes 2 new tests for weight decrease and heap upward restoration)
   - `remote / testOnly ...CompressionTableSpec`: 3/3 passed
   - `remote / testOnly ...OutboundCompressionSpec`: 2/2 passed
   
   ### References
   
   Refs akka/akka-core#31093


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [PR] feat: replace CountMinSketch with FastFrequencySketch in Artery compression [pekko]

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #3023:
URL: https://github.com/apache/pekko/pull/3023#issuecomment-4587817706

   I'd prefer if the old code was kept and made configurable


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [PR] feat: replace CountMinSketch with FastFrequencySketch in Artery compression [pekko]

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #3023:
URL: https://github.com/apache/pekko/pull/3023#discussion_r3420109998


##########
remote/src/main/resources/reference.conf:
##########
@@ -1046,6 +1046,12 @@ pekko {
         # compression of common strings in remoting messages, like actor destinations, serializers etc
         compression {
 
+          # Frequency sketch implementation used for heavy hitter detection.
+          # Options:
+          #   - "count-min-sketch": Original CountMinSketch implementation (legacy, ~128KB per connection)
+          #   - "fast-frequency-sketch": FastFrequencySketch with TinyLFU aging (default, ~4KB per connection)
+          frequency-sketch-implementation = "fast-frequency-sketch"

Review Comment:
   could we add an integration test that sets frequency-sketch-implementation = "count-min-sketch" ?



-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [PR] feat: replace CountMinSketch with FastFrequencySketch in Artery compression [pekko]

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #3023:
URL: https://github.com/apache/pekko/pull/3023#discussion_r3420109998


##########
remote/src/main/resources/reference.conf:
##########
@@ -1046,6 +1046,12 @@ pekko {
         # compression of common strings in remoting messages, like actor destinations, serializers etc
         compression {
 
+          # Frequency sketch implementation used for heavy hitter detection.
+          # Options:
+          #   - "count-min-sketch": Original CountMinSketch implementation (legacy, ~128KB per connection)
+          #   - "fast-frequency-sketch": FastFrequencySketch with TinyLFU aging (default, ~4KB per connection)
+          frequency-sketch-implementation = "fast-frequency-sketch"

Review Comment:
   could we add a unit test that sets frequency-sketch-implementation = "count-min-sketch" ?



-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [PR] feat: replace CountMinSketch with FastFrequencySketch in Artery compression [pekko]

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #3023:
URL: https://github.com/apache/pekko/pull/3023#discussion_r3330798355


##########
build.sbt:
##########
@@ -423,6 +424,9 @@ lazy val remote =
     .settings(OSGi.remote)
     .settings(Protobuf.settings)
     .settings(Test / parallelExecution := false)
+    .settings(mimaBinaryIssueFilters ++= Seq(
+      // CountMinSketch was INTERNAL API, replaced by FastFrequencySketch
+      ProblemFilters.exclude[MissingClassProblem]("org.apache.pekko.remote.artery.compress.CountMinSketch")))

Review Comment:
   can you move this to its own excludes file?



-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [PR] feat: replace CountMinSketch with FastFrequencySketch in Artery compression [pekko]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin merged PR #3023:
URL: https://github.com/apache/pekko/pull/3023


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [PR] feat: replace CountMinSketch with FastFrequencySketch in Artery compression [pekko]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #3023:
URL: https://github.com/apache/pekko/pull/3023#discussion_r3330814160


##########
build.sbt:
##########
@@ -423,6 +424,9 @@ lazy val remote =
     .settings(OSGi.remote)
     .settings(Protobuf.settings)
     .settings(Test / parallelExecution := false)
+    .settings(mimaBinaryIssueFilters ++= Seq(
+      // CountMinSketch was INTERNAL API, replaced by FastFrequencySketch
+      ProblemFilters.exclude[MissingClassProblem]("org.apache.pekko.remote.artery.compress.CountMinSketch")))

Review Comment:
   Yes, this should be done.



-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [PR] feat: replace CountMinSketch with FastFrequencySketch in Artery compression [pekko]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #3023:
URL: https://github.com/apache/pekko/pull/3023#discussion_r3419840666


##########
build.sbt:
##########
@@ -423,6 +424,9 @@ lazy val remote =
     .settings(OSGi.remote)
     .settings(Protobuf.settings)
     .settings(Test / parallelExecution := false)
+    .settings(mimaBinaryIssueFilters ++= Seq(
+      // CountMinSketch was INTERNAL API, replaced by FastFrequencySketch
+      ProblemFilters.exclude[MissingClassProblem]("org.apache.pekko.remote.artery.compress.CountMinSketch")))

Review Comment:
   The MiMa excludes were already removed in commit 83cd9ae3a6 — CountMinSketch is now kept as a configurable legacy option, so no MiMa exclusion is needed.



-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org