Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

[improvement] Use Direct ByteBuffers after upgrade to 2.8.x #1631

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

eolivelli
Copy link
Contributor

Motivation

With the upgrade to 2.8.0 we switched to use more heap and less DirectMemory.

Modifications

Use pooled Netty direct memory ByteBufs

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.
Need to update docs?

  • doc-required

    (If you need help on updating docs, create a doc issue)

  • no-need-doc

    (Please explain why)

  • doc

    (If this PR contains doc changes)

@github-actions github-actions bot added the no-need-doc This pr does not need any document label Dec 16, 2022
Copy link
Collaborator

@BewareMyPower BewareMyPower left a comment

Choose a reason for hiding this comment

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

The code itself LGTM. Please fix the code style.

@eolivelli eolivelli force-pushed the impl/kafka-client-use-bytebuf branch from 8de0f49 to 8e40ffe Compare March 7, 2023 12:57
@eolivelli eolivelli changed the title [improvement] Use pooled Direct ByteBufs after upgrade to 2.8.x [improvement] Use Direct ByteBuffers after upgrade to 2.8.x Mar 7, 2023
@eolivelli
Copy link
Contributor Author

@BewareMyPower @Demogorgon314 I have rebased and also applied a few more enhancement that we developed while running performance testing at very high rates (tens of millions of messages per second)

@eolivelli
Copy link
Contributor Author

some tests are failing, I will check them out later

@codecov
Copy link

codecov bot commented Mar 7, 2023

Codecov Report

Merging #1631 (0cb696d) into master (7faa3f4) will decrease coverage by 0.04%.
The diff coverage is 0.00%.

❗ Current head 0cb696d differs from pull request most recent head bed71a9. Consider uploading reports for the commit bed71a9 to get more accurate results

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1631      +/-   ##
============================================
- Coverage     16.05%   16.02%   -0.04%     
  Complexity      626      626              
============================================
  Files           165      165              
  Lines         12311    12338      +27     
  Branches       1128     1131       +3     
============================================
  Hits           1977     1977              
- Misses        10171    10198      +27     
  Partials        163      163              
Impacted Files Coverage Δ
...ative/pulsar/handlers/kop/KafkaCommandDecoder.java 0.32% <0.00%> (-0.02%) ⬇️
...ative/pulsar/handlers/kop/KafkaRequestHandler.java 1.08% <0.00%> (-0.01%) ⬇️
...rs/kop/coordinator/transaction/PendingRequest.java 0.00% <ø> (ø)
...r/transaction/TransactionMarkerChannelHandler.java 0.00% <0.00%> (ø)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
no-need-doc This pr does not need any document
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants