You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by Jiuming Tao <jm...@streamnative.io.INVALID> on 2022/02/23 09:42:47 UTC

[DISCUSS] PrometheusMetricsServlet performance improvement

Hi all,

1. I have learned that the /metrics endpoint will be requested by more than
one metrics collect system. In the condition, I want to reimplement
`PromethuesMetricsServlet` by sliding window.
PrometheusMetricsGenerator#generate will be invoked once in a period(such
as 1 minute), the result will be cached and returned for every metrics
collect request in the period directly. It could save memory and avoid high
CPU usage.

2. When there are hundreds MB metrics data collected, it causes high heap
memory usage, high CPU usage and GC pressure. In the
`PrometheusMetricsGenerator#generate` method, it uses
`ByteBufAllocator.DEFAULT.heapBuffer()` to allocate memory for writing
metrics data. The default size of `ByteBufAllocator.DEFAULT.heapBuffer()`
is 256 bytes, when the buffer resizes, the new buffer capacity is 512
bytes(power of 2) and with `mem_copy` operation.
If I want to write 100 MB data to the buffer, the current buffer size is
128 MB, and the total memory usage is close to 256 MB (256bytes + 512 bytes
+ 1k + .... + 64MB + 128MB). When the buffer size is greater than netty
buffer chunkSize(16 MB), it will be allocated as UnpooledHeapByteBuf in the
heap. After writing metrics data into the buffer, return it to the client
by jetty, jetty will copy it into jetty's buffer with memory allocation in
the heap, again!
In this condition, for the purpose of saving memory, avoid high CPU
usage(too much memory allocations and `mem_copy` operations) and reducing
GC pressure, I want to change `ByteBufAllocator.DEFAULT.heapBuffer()` to
`ByteBufAllocator.DEFAULT.compositeDirectBuffer()`, it wouldn't cause
`mem_copy` operations and huge memory allocations(CompositeDirectByteBuf is
a bit slowly in read/write, but it's worth). After writing data, I will
call the `HttpOutput#write(ByteBuffer)` method and write it to the client,
the method won't cause `mem_copy` (I have to wrap ByteBuf to ByteBuffer, if
ByteBuf wrapped, there will be zero-copy).
I tested NO.2 in my local, and it turns out performance is better than the
heap buffer(below images).

https://drive.google.com/file/d/1-0drrs9s9kZ2NbbVmzQDwPHdgtpE6QyW/view?usp=sharing
(CompositeDirectByteBuf)
https://drive.google.com/file/d/1-0m15YdsjBudsiweZ4DO7aU3bOFeK17w/view?usp=sharing
(PooledHeapByteBuf)

Thanks,
Tao Jiuming

Re: [DISCUSS] PrometheusMetricsServlet performance improvement

Posted by Christophe Bornet <bo...@gmail.com>.
Note that the PrometheusMetricsGenerator will generate the text for
Prometheus client metrics registered to the default CollectorRegistry.
So it should be possible to progressively convert stats objects to
Prometheus Gauge/Counter/etc...

Le mer. 9 mars 2022 à 06:57, Michael Marshall <mm...@apache.org> a
écrit :

> Thanks for looking into the prometheus client, Tao Jiuming. I am
> surprised that it isn't optimized to minimize heap usage. In that
> case, I think it makes sense to move forward with your solution.
>
> Thanks,
> Michael
>
>
>
> On Mon, Feb 28, 2022 at 10:23 PM PengHui Li <pe...@apache.org> wrote:
> >
> > Hi,
> >
> > Thanks for the update,
> > I agree to have the JVM heap usage enhancement.
> >
> > +1
> >
> > Penghui
> >
> > On Mon, Feb 28, 2022 at 5:33 PM Jiuming Tao
> <jm...@streamnative.io.invalid>
> > wrote:
> >
> > > Hi Penghui,
> > >
> > > I had read `io.prometheus.client.exporter.HttpServer` source code, in
> > > `HttpMetricsHandler#handle` method, it uses thread local cached
> > > `ByteArrayOutputStream` , it’s similar with our current
> implemention(with
> > > heap memory resizes and mem_copy).
> > > It will spend a plenty of heap memory, and even worse, these heap
> memory
> > > will never be released(cached in thread local).
> > >
> > > Thanks,
> > > Tao Jiuming
> > >
> > > > 2022年2月28日 下午5:00,PengHui Li <pe...@apache.org> 写道:
> > > >
> > > > Hi Jiuming,
> > > >
> > > > Could you please check if the Prometheus client
> > > > can be used to reduce the JVM heap usage?
> > > > If yes, I think we can consider using the Prometheus
> > > > client instead of the current implementation together.
> > > > Otherwise, we'd better focus on the heap memory usage
> > > > enhancement for this discussion. Using the Prometheus
> > > > client to refactor the current implementation will be a
> > > > big project.
> > > >
> > > > Thanks,
> > > > Penghui
> > > >
> > > > On Sun, Feb 27, 2022 at 12:22 AM Jiuming Tao
> > > <jm...@streamnative.io.invalid>
> > > > wrote:
> > > >
> > > >> Hi all,
> > > >> https://github.com/apache/pulsar/pull/14453 <
> > > >> https://github.com/apache/pulsar/pull/14453>  please take a look.
> > > >>
> > > >> Thanks,
> > > >> Tao Jiuming
> > > >>
> > > >>> 2022年2月24日 上午1:05,Jiuming Tao <jm...@streamnative.io> 写道:
> > > >>>
> > > >>> Hi all,
> > > >>>>
> > > >>>> 2. When there are hundreds MB metrics data collected, it causes
> high
> > > >> heap memory usage, high CPU usage and GC pressure. In the
> > > >> `PrometheusMetricsGenerator#generate` method, it uses
> > > >> `ByteBufAllocator.DEFAULT.heapBuffer()` to allocate memory for
> writing
> > > >> metrics data. The default size of
> > > `ByteBufAllocator.DEFAULT.heapBuffer()`
> > > >> is 256 bytes, when the buffer resizes, the new buffer capacity is
> 512
> > > >> bytes(power of 2) and with `mem_copy` operation.
> > > >>>> If I want to write 100 MB data to the buffer, the current buffer
> size
> > > >> is 128 MB, and the total memory usage is close to 256 MB (256bytes
> + 512
> > > >> bytes + 1k + .... + 64MB + 128MB). When the buffer size is greater
> than
> > > >> netty buffer chunkSize(16 MB), it will be allocated as
> > > UnpooledHeapByteBuf
> > > >> in the heap. After writing metrics data into the buffer, return it
> to
> > > the
> > > >> client by jetty, jetty will copy it into jetty's buffer with memory
> > > >> allocation in the heap, again!
> > > >>>> In this condition, for the purpose of saving memory, avoid high
> CPU
> > > >> usage(too much memory allocations and `mem_copy` operations) and
> > > reducing
> > > >> GC pressure, I want to change
> `ByteBufAllocator.DEFAULT.heapBuffer()` to
> > > >> `ByteBufAllocator.DEFAULT.compositeDirectBuffer()`, it wouldn't
> cause
> > > >> `mem_copy` operations and huge memory
> > > allocations(CompositeDirectByteBuf is
> > > >> a bit slowly in read/write, but it's worth). After writing data, I
> will
> > > >> call the `HttpOutput#write(ByteBuffer)` method and write it to the
> > > client,
> > > >> the method won't cause `mem_copy` (I have to wrap ByteBuf to
> > > ByteBuffer, if
> > > >> ByteBuf wrapped, there will be zero-copy).
> > > >>>
> > > >>> The jdk in my local is jdk15, I just noticed that in jdk8,
> ByteBuffer
> > > >> cannot be extended and implemented. So, if allowed, I will write
> metrics
> > > >> data to temp files and send it to client by jetty’s send_file. It
> will
> > > be
> > > >> turned out a better performance than `CompositeByteBuf`, and takes
> lower
> > > >> CPU usage due to I/O blocking.(The /metrics endpoint will be a bit
> > > slowly,
> > > >> I believe it’s worth).
> > > >>> If not allowed, it’s no matter and it also has a better performance
> > > than
> > > >> `ByteBufAllocator.DEFAULT.heapBuffer()`(see the first image in
> original
> > > >> mail).
> > > >>>
> > > >>> Thanks,
> > > >>> Tao Jiuming
> > > >>
> > > >>
> > >
> > >
>

Re: [DISCUSS] PrometheusMetricsServlet performance improvement

Posted by Michael Marshall <mm...@apache.org>.
Thanks for looking into the prometheus client, Tao Jiuming. I am
surprised that it isn't optimized to minimize heap usage. In that
case, I think it makes sense to move forward with your solution.

Thanks,
Michael



On Mon, Feb 28, 2022 at 10:23 PM PengHui Li <pe...@apache.org> wrote:
>
> Hi,
>
> Thanks for the update,
> I agree to have the JVM heap usage enhancement.
>
> +1
>
> Penghui
>
> On Mon, Feb 28, 2022 at 5:33 PM Jiuming Tao <jm...@streamnative.io.invalid>
> wrote:
>
> > Hi Penghui,
> >
> > I had read `io.prometheus.client.exporter.HttpServer` source code, in
> > `HttpMetricsHandler#handle` method, it uses thread local cached
> > `ByteArrayOutputStream` , it’s similar with our current implemention(with
> > heap memory resizes and mem_copy).
> > It will spend a plenty of heap memory, and even worse, these heap memory
> > will never be released(cached in thread local).
> >
> > Thanks,
> > Tao Jiuming
> >
> > > 2022年2月28日 下午5:00,PengHui Li <pe...@apache.org> 写道:
> > >
> > > Hi Jiuming,
> > >
> > > Could you please check if the Prometheus client
> > > can be used to reduce the JVM heap usage?
> > > If yes, I think we can consider using the Prometheus
> > > client instead of the current implementation together.
> > > Otherwise, we'd better focus on the heap memory usage
> > > enhancement for this discussion. Using the Prometheus
> > > client to refactor the current implementation will be a
> > > big project.
> > >
> > > Thanks,
> > > Penghui
> > >
> > > On Sun, Feb 27, 2022 at 12:22 AM Jiuming Tao
> > <jm...@streamnative.io.invalid>
> > > wrote:
> > >
> > >> Hi all,
> > >> https://github.com/apache/pulsar/pull/14453 <
> > >> https://github.com/apache/pulsar/pull/14453>  please take a look.
> > >>
> > >> Thanks,
> > >> Tao Jiuming
> > >>
> > >>> 2022年2月24日 上午1:05,Jiuming Tao <jm...@streamnative.io> 写道:
> > >>>
> > >>> Hi all,
> > >>>>
> > >>>> 2. When there are hundreds MB metrics data collected, it causes high
> > >> heap memory usage, high CPU usage and GC pressure. In the
> > >> `PrometheusMetricsGenerator#generate` method, it uses
> > >> `ByteBufAllocator.DEFAULT.heapBuffer()` to allocate memory for writing
> > >> metrics data. The default size of
> > `ByteBufAllocator.DEFAULT.heapBuffer()`
> > >> is 256 bytes, when the buffer resizes, the new buffer capacity is 512
> > >> bytes(power of 2) and with `mem_copy` operation.
> > >>>> If I want to write 100 MB data to the buffer, the current buffer size
> > >> is 128 MB, and the total memory usage is close to 256 MB (256bytes + 512
> > >> bytes + 1k + .... + 64MB + 128MB). When the buffer size is greater than
> > >> netty buffer chunkSize(16 MB), it will be allocated as
> > UnpooledHeapByteBuf
> > >> in the heap. After writing metrics data into the buffer, return it to
> > the
> > >> client by jetty, jetty will copy it into jetty's buffer with memory
> > >> allocation in the heap, again!
> > >>>> In this condition, for the purpose of saving memory, avoid high CPU
> > >> usage(too much memory allocations and `mem_copy` operations) and
> > reducing
> > >> GC pressure, I want to change `ByteBufAllocator.DEFAULT.heapBuffer()` to
> > >> `ByteBufAllocator.DEFAULT.compositeDirectBuffer()`, it wouldn't cause
> > >> `mem_copy` operations and huge memory
> > allocations(CompositeDirectByteBuf is
> > >> a bit slowly in read/write, but it's worth). After writing data, I will
> > >> call the `HttpOutput#write(ByteBuffer)` method and write it to the
> > client,
> > >> the method won't cause `mem_copy` (I have to wrap ByteBuf to
> > ByteBuffer, if
> > >> ByteBuf wrapped, there will be zero-copy).
> > >>>
> > >>> The jdk in my local is jdk15, I just noticed that in jdk8, ByteBuffer
> > >> cannot be extended and implemented. So, if allowed, I will write metrics
> > >> data to temp files and send it to client by jetty’s send_file. It will
> > be
> > >> turned out a better performance than `CompositeByteBuf`, and takes lower
> > >> CPU usage due to I/O blocking.(The /metrics endpoint will be a bit
> > slowly,
> > >> I believe it’s worth).
> > >>> If not allowed, it’s no matter and it also has a better performance
> > than
> > >> `ByteBufAllocator.DEFAULT.heapBuffer()`(see the first image in original
> > >> mail).
> > >>>
> > >>> Thanks,
> > >>> Tao Jiuming
> > >>
> > >>
> >
> >

Re: [DISCUSS] PrometheusMetricsServlet performance improvement

Posted by PengHui Li <pe...@apache.org>.
Hi,

Thanks for the update,
I agree to have the JVM heap usage enhancement.

+1

Penghui

On Mon, Feb 28, 2022 at 5:33 PM Jiuming Tao <jm...@streamnative.io.invalid>
wrote:

> Hi Penghui,
>
> I had read `io.prometheus.client.exporter.HttpServer` source code, in
> `HttpMetricsHandler#handle` method, it uses thread local cached
> `ByteArrayOutputStream` , it’s similar with our current implemention(with
> heap memory resizes and mem_copy).
> It will spend a plenty of heap memory, and even worse, these heap memory
> will never be released(cached in thread local).
>
> Thanks,
> Tao Jiuming
>
> > 2022年2月28日 下午5:00,PengHui Li <pe...@apache.org> 写道:
> >
> > Hi Jiuming,
> >
> > Could you please check if the Prometheus client
> > can be used to reduce the JVM heap usage?
> > If yes, I think we can consider using the Prometheus
> > client instead of the current implementation together.
> > Otherwise, we'd better focus on the heap memory usage
> > enhancement for this discussion. Using the Prometheus
> > client to refactor the current implementation will be a
> > big project.
> >
> > Thanks,
> > Penghui
> >
> > On Sun, Feb 27, 2022 at 12:22 AM Jiuming Tao
> <jm...@streamnative.io.invalid>
> > wrote:
> >
> >> Hi all,
> >> https://github.com/apache/pulsar/pull/14453 <
> >> https://github.com/apache/pulsar/pull/14453>  please take a look.
> >>
> >> Thanks,
> >> Tao Jiuming
> >>
> >>> 2022年2月24日 上午1:05,Jiuming Tao <jm...@streamnative.io> 写道:
> >>>
> >>> Hi all,
> >>>>
> >>>> 2. When there are hundreds MB metrics data collected, it causes high
> >> heap memory usage, high CPU usage and GC pressure. In the
> >> `PrometheusMetricsGenerator#generate` method, it uses
> >> `ByteBufAllocator.DEFAULT.heapBuffer()` to allocate memory for writing
> >> metrics data. The default size of
> `ByteBufAllocator.DEFAULT.heapBuffer()`
> >> is 256 bytes, when the buffer resizes, the new buffer capacity is 512
> >> bytes(power of 2) and with `mem_copy` operation.
> >>>> If I want to write 100 MB data to the buffer, the current buffer size
> >> is 128 MB, and the total memory usage is close to 256 MB (256bytes + 512
> >> bytes + 1k + .... + 64MB + 128MB). When the buffer size is greater than
> >> netty buffer chunkSize(16 MB), it will be allocated as
> UnpooledHeapByteBuf
> >> in the heap. After writing metrics data into the buffer, return it to
> the
> >> client by jetty, jetty will copy it into jetty's buffer with memory
> >> allocation in the heap, again!
> >>>> In this condition, for the purpose of saving memory, avoid high CPU
> >> usage(too much memory allocations and `mem_copy` operations) and
> reducing
> >> GC pressure, I want to change `ByteBufAllocator.DEFAULT.heapBuffer()` to
> >> `ByteBufAllocator.DEFAULT.compositeDirectBuffer()`, it wouldn't cause
> >> `mem_copy` operations and huge memory
> allocations(CompositeDirectByteBuf is
> >> a bit slowly in read/write, but it's worth). After writing data, I will
> >> call the `HttpOutput#write(ByteBuffer)` method and write it to the
> client,
> >> the method won't cause `mem_copy` (I have to wrap ByteBuf to
> ByteBuffer, if
> >> ByteBuf wrapped, there will be zero-copy).
> >>>
> >>> The jdk in my local is jdk15, I just noticed that in jdk8, ByteBuffer
> >> cannot be extended and implemented. So, if allowed, I will write metrics
> >> data to temp files and send it to client by jetty’s send_file. It will
> be
> >> turned out a better performance than `CompositeByteBuf`, and takes lower
> >> CPU usage due to I/O blocking.(The /metrics endpoint will be a bit
> slowly,
> >> I believe it’s worth).
> >>> If not allowed, it’s no matter and it also has a better performance
> than
> >> `ByteBufAllocator.DEFAULT.heapBuffer()`(see the first image in original
> >> mail).
> >>>
> >>> Thanks,
> >>> Tao Jiuming
> >>
> >>
>
>

Re: [DISCUSS] PrometheusMetricsServlet performance improvement

Posted by Jiuming Tao <jm...@streamnative.io.INVALID>.
Hi Penghui,

I had read `io.prometheus.client.exporter.HttpServer` source code, in `HttpMetricsHandler#handle` method, it uses thread local cached `ByteArrayOutputStream` , it’s similar with our current implemention(with heap memory resizes and mem_copy). 
It will spend a plenty of heap memory, and even worse, these heap memory will never be released(cached in thread local).

Thanks,
Tao Jiuming

> 2022年2月28日 下午5:00,PengHui Li <pe...@apache.org> 写道:
> 
> Hi Jiuming,
> 
> Could you please check if the Prometheus client
> can be used to reduce the JVM heap usage?
> If yes, I think we can consider using the Prometheus
> client instead of the current implementation together.
> Otherwise, we'd better focus on the heap memory usage
> enhancement for this discussion. Using the Prometheus
> client to refactor the current implementation will be a
> big project.
> 
> Thanks,
> Penghui
> 
> On Sun, Feb 27, 2022 at 12:22 AM Jiuming Tao <jm...@streamnative.io.invalid>
> wrote:
> 
>> Hi all,
>> https://github.com/apache/pulsar/pull/14453 <
>> https://github.com/apache/pulsar/pull/14453>  please take a look.
>> 
>> Thanks,
>> Tao Jiuming
>> 
>>> 2022年2月24日 上午1:05,Jiuming Tao <jm...@streamnative.io> 写道:
>>> 
>>> Hi all,
>>>> 
>>>> 2. When there are hundreds MB metrics data collected, it causes high
>> heap memory usage, high CPU usage and GC pressure. In the
>> `PrometheusMetricsGenerator#generate` method, it uses
>> `ByteBufAllocator.DEFAULT.heapBuffer()` to allocate memory for writing
>> metrics data. The default size of `ByteBufAllocator.DEFAULT.heapBuffer()`
>> is 256 bytes, when the buffer resizes, the new buffer capacity is 512
>> bytes(power of 2) and with `mem_copy` operation.
>>>> If I want to write 100 MB data to the buffer, the current buffer size
>> is 128 MB, and the total memory usage is close to 256 MB (256bytes + 512
>> bytes + 1k + .... + 64MB + 128MB). When the buffer size is greater than
>> netty buffer chunkSize(16 MB), it will be allocated as UnpooledHeapByteBuf
>> in the heap. After writing metrics data into the buffer, return it to the
>> client by jetty, jetty will copy it into jetty's buffer with memory
>> allocation in the heap, again!
>>>> In this condition, for the purpose of saving memory, avoid high CPU
>> usage(too much memory allocations and `mem_copy` operations) and reducing
>> GC pressure, I want to change `ByteBufAllocator.DEFAULT.heapBuffer()` to
>> `ByteBufAllocator.DEFAULT.compositeDirectBuffer()`, it wouldn't cause
>> `mem_copy` operations and huge memory allocations(CompositeDirectByteBuf is
>> a bit slowly in read/write, but it's worth). After writing data, I will
>> call the `HttpOutput#write(ByteBuffer)` method and write it to the client,
>> the method won't cause `mem_copy` (I have to wrap ByteBuf to ByteBuffer, if
>> ByteBuf wrapped, there will be zero-copy).
>>> 
>>> The jdk in my local is jdk15, I just noticed that in jdk8, ByteBuffer
>> cannot be extended and implemented. So, if allowed, I will write metrics
>> data to temp files and send it to client by jetty’s send_file. It will be
>> turned out a better performance than `CompositeByteBuf`, and takes lower
>> CPU usage due to I/O blocking.(The /metrics endpoint will be a bit slowly,
>> I believe it’s worth).
>>> If not allowed, it’s no matter and it also has a better performance than
>> `ByteBufAllocator.DEFAULT.heapBuffer()`(see the first image in original
>> mail).
>>> 
>>> Thanks,
>>> Tao Jiuming
>> 
>> 


Re: [DISCUSS] PrometheusMetricsServlet performance improvement

Posted by PengHui Li <pe...@apache.org>.
Hi Jiuming,

Could you please check if the Prometheus client
can be used to reduce the JVM heap usage?
If yes, I think we can consider using the Prometheus
client instead of the current implementation together.
Otherwise, we'd better focus on the heap memory usage
enhancement for this discussion. Using the Prometheus
client to refactor the current implementation will be a
big project.

Thanks,
Penghui

On Sun, Feb 27, 2022 at 12:22 AM Jiuming Tao <jm...@streamnative.io.invalid>
wrote:

> Hi all,
> https://github.com/apache/pulsar/pull/14453 <
> https://github.com/apache/pulsar/pull/14453>  please take a look.
>
> Thanks,
> Tao Jiuming
>
> > 2022年2月24日 上午1:05,Jiuming Tao <jm...@streamnative.io> 写道:
> >
> > Hi all,
> >>
> >> 2. When there are hundreds MB metrics data collected, it causes high
> heap memory usage, high CPU usage and GC pressure. In the
> `PrometheusMetricsGenerator#generate` method, it uses
> `ByteBufAllocator.DEFAULT.heapBuffer()` to allocate memory for writing
> metrics data. The default size of `ByteBufAllocator.DEFAULT.heapBuffer()`
> is 256 bytes, when the buffer resizes, the new buffer capacity is 512
> bytes(power of 2) and with `mem_copy` operation.
> >> If I want to write 100 MB data to the buffer, the current buffer size
> is 128 MB, and the total memory usage is close to 256 MB (256bytes + 512
> bytes + 1k + .... + 64MB + 128MB). When the buffer size is greater than
> netty buffer chunkSize(16 MB), it will be allocated as UnpooledHeapByteBuf
> in the heap. After writing metrics data into the buffer, return it to the
> client by jetty, jetty will copy it into jetty's buffer with memory
> allocation in the heap, again!
> >> In this condition, for the purpose of saving memory, avoid high CPU
> usage(too much memory allocations and `mem_copy` operations) and reducing
> GC pressure, I want to change `ByteBufAllocator.DEFAULT.heapBuffer()` to
> `ByteBufAllocator.DEFAULT.compositeDirectBuffer()`, it wouldn't cause
> `mem_copy` operations and huge memory allocations(CompositeDirectByteBuf is
> a bit slowly in read/write, but it's worth). After writing data, I will
> call the `HttpOutput#write(ByteBuffer)` method and write it to the client,
> the method won't cause `mem_copy` (I have to wrap ByteBuf to ByteBuffer, if
> ByteBuf wrapped, there will be zero-copy).
> >
> > The jdk in my local is jdk15, I just noticed that in jdk8, ByteBuffer
> cannot be extended and implemented. So, if allowed, I will write metrics
> data to temp files and send it to client by jetty’s send_file. It will be
> turned out a better performance than `CompositeByteBuf`, and takes lower
> CPU usage due to I/O blocking.(The /metrics endpoint will be a bit slowly,
> I believe it’s worth).
> > If not allowed, it’s no matter and it also has a better performance than
> `ByteBufAllocator.DEFAULT.heapBuffer()`(see the first image in original
> mail).
> >
> > Thanks,
> > Tao Jiuming
>
>

Re: [DISCUSS] PrometheusMetricsServlet performance improvement

Posted by Jiuming Tao <jm...@streamnative.io.INVALID>.
Hi all,
https://github.com/apache/pulsar/pull/14453 <https://github.com/apache/pulsar/pull/14453>  please take a look.

Thanks,
Tao Jiuming

> 2022年2月24日 上午1:05,Jiuming Tao <jm...@streamnative.io> 写道:
> 
> Hi all,
>> 
>> 2. When there are hundreds MB metrics data collected, it causes high heap memory usage, high CPU usage and GC pressure. In the `PrometheusMetricsGenerator#generate` method, it uses `ByteBufAllocator.DEFAULT.heapBuffer()` to allocate memory for writing metrics data. The default size of `ByteBufAllocator.DEFAULT.heapBuffer()` is 256 bytes, when the buffer resizes, the new buffer capacity is 512 bytes(power of 2) and with `mem_copy` operation.
>> If I want to write 100 MB data to the buffer, the current buffer size is 128 MB, and the total memory usage is close to 256 MB (256bytes + 512 bytes + 1k + .... + 64MB + 128MB). When the buffer size is greater than netty buffer chunkSize(16 MB), it will be allocated as UnpooledHeapByteBuf in the heap. After writing metrics data into the buffer, return it to the client by jetty, jetty will copy it into jetty's buffer with memory allocation in the heap, again!
>> In this condition, for the purpose of saving memory, avoid high CPU usage(too much memory allocations and `mem_copy` operations) and reducing GC pressure, I want to change `ByteBufAllocator.DEFAULT.heapBuffer()` to `ByteBufAllocator.DEFAULT.compositeDirectBuffer()`, it wouldn't cause `mem_copy` operations and huge memory allocations(CompositeDirectByteBuf is a bit slowly in read/write, but it's worth). After writing data, I will call the `HttpOutput#write(ByteBuffer)` method and write it to the client, the method won't cause `mem_copy` (I have to wrap ByteBuf to ByteBuffer, if ByteBuf wrapped, there will be zero-copy).
> 
> The jdk in my local is jdk15, I just noticed that in jdk8, ByteBuffer cannot be extended and implemented. So, if allowed, I will write metrics data to temp files and send it to client by jetty’s send_file. It will be turned out a better performance than `CompositeByteBuf`, and takes lower CPU usage due to I/O blocking.(The /metrics endpoint will be a bit slowly, I believe it’s worth).
> If not allowed, it’s no matter and it also has a better performance than `ByteBufAllocator.DEFAULT.heapBuffer()`(see the first image in original mail). 
> 
> Thanks,
> Tao Jiuming


Re: [DISCUSS] PrometheusMetricsServlet performance improvement

Posted by Michael Marshall <mm...@apache.org>.
> Old codes, I think at that time, prometheus is not popular yet

I think there is likely more explanation here, since we could have
switched any time in the past few years when prometheus was already
popular.

Before we add caching to our metrics generation, I think we should
consider migrating to the prometheus client. I can't tell from the
prometheus client documentation whether the client has this caching
feature. If it does, then that is an easy win. If it does, I wonder if
that implies that premetheus endpoints are not meant to be queried too
frequently.

> In cloud, maybe cloud services providers monitors the cluster, and users also monitors it.

Are you able to provide more detail about which cloud service
providers? Is this just a prometheus server scraping metrics?
Regarding users, I would recommend they view prometheus metrics via
prometheus/grafana precisely because it will decrease load on the broker.
I don't mean to be too pedantic, but this whole feature relies on the
premise that brokers are handling frequent calls to the /metrics
endpoint, so I would like to understand the motivation.

Thanks,
Michael



On Thu, Feb 24, 2022 at 10:48 PM Jiuming Tao
<jm...@streamnative.io.invalid> wrote:
>
> > I have a historical question. Why do we write and maintain our own
> > code to generate the metrics response instead of using the prometheus
> > client library?
>
> Old codes, I think at that time, prometheus is not popular yet
>
>
> >> I have learned that the /metrics endpoint will be requested by more than
> >> one metrics collect system.
> >
> > In practice, when does this happen?
> In cloud, maybe cloud services providers monitors the cluster, and users also monitors it.
>
> >> PrometheusMetricsGenerator#generate will be invoked once in a period(such
> >> as 1 minute), the result will be cached and returned for every metrics
> >> collect request in the period directly.
> >
> > Since there are tradeoffs to the cache duration, we should make the
> > period configurable.
>
> Yes, of course
>
> > 2022年2月25日 下午12:41,Michael Marshall <mm...@apache.org> 写道:
> >
> > I have a historical question. Why do we write and maintain our own
> > code to generate the metrics response instead of using the prometheus
> > client library?
> >
> >> I have learned that the /metrics endpoint will be requested by more than
> >> one metrics collect system.
> >
> > In practice, when does this happen?
> >
> >> PrometheusMetricsGenerator#generate will be invoked once in a period(such
> >> as 1 minute), the result will be cached and returned for every metrics
> >> collect request in the period directly.
> >
> > Since there are tradeoffs to the cache duration, we should make the
> > period configurable.
> >
> > Thanks,
> > Michael
> >
> > On Wed, Feb 23, 2022 at 11:06 AM Jiuming Tao
> > <jm...@streamnative.io.invalid> wrote:
> >>
> >> Hi all,
> >>>
> >>> 2. When there are hundreds MB metrics data collected, it causes high heap memory usage, high CPU usage and GC pressure. In the `PrometheusMetricsGenerator#generate` method, it uses `ByteBufAllocator.DEFAULT.heapBuffer()` to allocate memory for writing metrics data. The default size of `ByteBufAllocator.DEFAULT.heapBuffer()` is 256 bytes, when the buffer resizes, the new buffer capacity is 512 bytes(power of 2) and with `mem_copy` operation.
> >>> If I want to write 100 MB data to the buffer, the current buffer size is 128 MB, and the total memory usage is close to 256 MB (256bytes + 512 bytes + 1k + .... + 64MB + 128MB). When the buffer size is greater than netty buffer chunkSize(16 MB), it will be allocated as UnpooledHeapByteBuf in the heap. After writing metrics data into the buffer, return it to the client by jetty, jetty will copy it into jetty's buffer with memory allocation in the heap, again!
> >>> In this condition, for the purpose of saving memory, avoid high CPU usage(too much memory allocations and `mem_copy` operations) and reducing GC pressure, I want to change `ByteBufAllocator.DEFAULT.heapBuffer()` to `ByteBufAllocator.DEFAULT.compositeDirectBuffer()`, it wouldn't cause `mem_copy` operations and huge memory allocations(CompositeDirectByteBuf is a bit slowly in read/write, but it's worth). After writing data, I will call the `HttpOutput#write(ByteBuffer)` method and write it to the client, the method won't cause `mem_copy` (I have to wrap ByteBuf to ByteBuffer, if ByteBuf wrapped, there will be zero-copy).
> >>
> >> The jdk in my local is jdk15, I just noticed that in jdk8, ByteBuffer cannot be extended and implemented. So, if allowed, I will write metrics data to temp files and send it to client by jetty’s send_file. It will be turned out a better performance than `CompositeByteBuf`, and takes lower CPU usage due to I/O blocking.(The /metrics endpoint will be a bit slowly, I believe it’s worth).
> >> If not allowed, it’s no matter and it also has a better performance than `ByteBufAllocator.DEFAULT.heapBuffer()`(see the first image in original mail).
> >>
> >> Thanks,
> >> Tao Jiuming
>

Re: [DISCUSS] PrometheusMetricsServlet performance improvement

Posted by Jiuming Tao <jm...@streamnative.io.INVALID>.
> I have a historical question. Why do we write and maintain our own
> code to generate the metrics response instead of using the prometheus
> client library?

Old codes, I think at that time, prometheus is not popular yet


>> I have learned that the /metrics endpoint will be requested by more than
>> one metrics collect system.
> 
> In practice, when does this happen?
In cloud, maybe cloud services providers monitors the cluster, and users also monitors it.

>> PrometheusMetricsGenerator#generate will be invoked once in a period(such
>> as 1 minute), the result will be cached and returned for every metrics
>> collect request in the period directly.
> 
> Since there are tradeoffs to the cache duration, we should make the
> period configurable.

Yes, of course

> 2022年2月25日 下午12:41,Michael Marshall <mm...@apache.org> 写道:
> 
> I have a historical question. Why do we write and maintain our own
> code to generate the metrics response instead of using the prometheus
> client library?
> 
>> I have learned that the /metrics endpoint will be requested by more than
>> one metrics collect system.
> 
> In practice, when does this happen?
> 
>> PrometheusMetricsGenerator#generate will be invoked once in a period(such
>> as 1 minute), the result will be cached and returned for every metrics
>> collect request in the period directly.
> 
> Since there are tradeoffs to the cache duration, we should make the
> period configurable.
> 
> Thanks,
> Michael
> 
> On Wed, Feb 23, 2022 at 11:06 AM Jiuming Tao
> <jm...@streamnative.io.invalid> wrote:
>> 
>> Hi all,
>>> 
>>> 2. When there are hundreds MB metrics data collected, it causes high heap memory usage, high CPU usage and GC pressure. In the `PrometheusMetricsGenerator#generate` method, it uses `ByteBufAllocator.DEFAULT.heapBuffer()` to allocate memory for writing metrics data. The default size of `ByteBufAllocator.DEFAULT.heapBuffer()` is 256 bytes, when the buffer resizes, the new buffer capacity is 512 bytes(power of 2) and with `mem_copy` operation.
>>> If I want to write 100 MB data to the buffer, the current buffer size is 128 MB, and the total memory usage is close to 256 MB (256bytes + 512 bytes + 1k + .... + 64MB + 128MB). When the buffer size is greater than netty buffer chunkSize(16 MB), it will be allocated as UnpooledHeapByteBuf in the heap. After writing metrics data into the buffer, return it to the client by jetty, jetty will copy it into jetty's buffer with memory allocation in the heap, again!
>>> In this condition, for the purpose of saving memory, avoid high CPU usage(too much memory allocations and `mem_copy` operations) and reducing GC pressure, I want to change `ByteBufAllocator.DEFAULT.heapBuffer()` to `ByteBufAllocator.DEFAULT.compositeDirectBuffer()`, it wouldn't cause `mem_copy` operations and huge memory allocations(CompositeDirectByteBuf is a bit slowly in read/write, but it's worth). After writing data, I will call the `HttpOutput#write(ByteBuffer)` method and write it to the client, the method won't cause `mem_copy` (I have to wrap ByteBuf to ByteBuffer, if ByteBuf wrapped, there will be zero-copy).
>> 
>> The jdk in my local is jdk15, I just noticed that in jdk8, ByteBuffer cannot be extended and implemented. So, if allowed, I will write metrics data to temp files and send it to client by jetty’s send_file. It will be turned out a better performance than `CompositeByteBuf`, and takes lower CPU usage due to I/O blocking.(The /metrics endpoint will be a bit slowly, I believe it’s worth).
>> If not allowed, it’s no matter and it also has a better performance than `ByteBufAllocator.DEFAULT.heapBuffer()`(see the first image in original mail).
>> 
>> Thanks,
>> Tao Jiuming


Re: [DISCUSS] PrometheusMetricsServlet performance improvement

Posted by Michael Marshall <mm...@apache.org>.
I have a historical question. Why do we write and maintain our own
code to generate the metrics response instead of using the prometheus
client library?

> I have learned that the /metrics endpoint will be requested by more than
> one metrics collect system.

In practice, when does this happen?

> PrometheusMetricsGenerator#generate will be invoked once in a period(such
> as 1 minute), the result will be cached and returned for every metrics
> collect request in the period directly.

Since there are tradeoffs to the cache duration, we should make the
period configurable.

Thanks,
Michael

On Wed, Feb 23, 2022 at 11:06 AM Jiuming Tao
<jm...@streamnative.io.invalid> wrote:
>
> Hi all,
> >
> > 2. When there are hundreds MB metrics data collected, it causes high heap memory usage, high CPU usage and GC pressure. In the `PrometheusMetricsGenerator#generate` method, it uses `ByteBufAllocator.DEFAULT.heapBuffer()` to allocate memory for writing metrics data. The default size of `ByteBufAllocator.DEFAULT.heapBuffer()` is 256 bytes, when the buffer resizes, the new buffer capacity is 512 bytes(power of 2) and with `mem_copy` operation.
> > If I want to write 100 MB data to the buffer, the current buffer size is 128 MB, and the total memory usage is close to 256 MB (256bytes + 512 bytes + 1k + .... + 64MB + 128MB). When the buffer size is greater than netty buffer chunkSize(16 MB), it will be allocated as UnpooledHeapByteBuf in the heap. After writing metrics data into the buffer, return it to the client by jetty, jetty will copy it into jetty's buffer with memory allocation in the heap, again!
> > In this condition, for the purpose of saving memory, avoid high CPU usage(too much memory allocations and `mem_copy` operations) and reducing GC pressure, I want to change `ByteBufAllocator.DEFAULT.heapBuffer()` to `ByteBufAllocator.DEFAULT.compositeDirectBuffer()`, it wouldn't cause `mem_copy` operations and huge memory allocations(CompositeDirectByteBuf is a bit slowly in read/write, but it's worth). After writing data, I will call the `HttpOutput#write(ByteBuffer)` method and write it to the client, the method won't cause `mem_copy` (I have to wrap ByteBuf to ByteBuffer, if ByteBuf wrapped, there will be zero-copy).
>
> The jdk in my local is jdk15, I just noticed that in jdk8, ByteBuffer cannot be extended and implemented. So, if allowed, I will write metrics data to temp files and send it to client by jetty’s send_file. It will be turned out a better performance than `CompositeByteBuf`, and takes lower CPU usage due to I/O blocking.(The /metrics endpoint will be a bit slowly, I believe it’s worth).
> If not allowed, it’s no matter and it also has a better performance than `ByteBufAllocator.DEFAULT.heapBuffer()`(see the first image in original mail).
>
> Thanks,
> Tao Jiuming

Re: [DISCUSS] PrometheusMetricsServlet performance improvement

Posted by Jiuming Tao <jm...@streamnative.io.INVALID>.
Hi all,
> 
> 2. When there are hundreds MB metrics data collected, it causes high heap memory usage, high CPU usage and GC pressure. In the `PrometheusMetricsGenerator#generate` method, it uses `ByteBufAllocator.DEFAULT.heapBuffer()` to allocate memory for writing metrics data. The default size of `ByteBufAllocator.DEFAULT.heapBuffer()` is 256 bytes, when the buffer resizes, the new buffer capacity is 512 bytes(power of 2) and with `mem_copy` operation.
> If I want to write 100 MB data to the buffer, the current buffer size is 128 MB, and the total memory usage is close to 256 MB (256bytes + 512 bytes + 1k + .... + 64MB + 128MB). When the buffer size is greater than netty buffer chunkSize(16 MB), it will be allocated as UnpooledHeapByteBuf in the heap. After writing metrics data into the buffer, return it to the client by jetty, jetty will copy it into jetty's buffer with memory allocation in the heap, again!
> In this condition, for the purpose of saving memory, avoid high CPU usage(too much memory allocations and `mem_copy` operations) and reducing GC pressure, I want to change `ByteBufAllocator.DEFAULT.heapBuffer()` to `ByteBufAllocator.DEFAULT.compositeDirectBuffer()`, it wouldn't cause `mem_copy` operations and huge memory allocations(CompositeDirectByteBuf is a bit slowly in read/write, but it's worth). After writing data, I will call the `HttpOutput#write(ByteBuffer)` method and write it to the client, the method won't cause `mem_copy` (I have to wrap ByteBuf to ByteBuffer, if ByteBuf wrapped, there will be zero-copy).

The jdk in my local is jdk15, I just noticed that in jdk8, ByteBuffer cannot be extended and implemented. So, if allowed, I will write metrics data to temp files and send it to client by jetty’s send_file. It will be turned out a better performance than `CompositeByteBuf`, and takes lower CPU usage due to I/O blocking.(The /metrics endpoint will be a bit slowly, I believe it’s worth).
If not allowed, it’s no matter and it also has a better performance than `ByteBufAllocator.DEFAULT.heapBuffer()`(see the first image in original mail). 

Thanks,
Tao Jiuming

Re: [DISCUSS] PrometheusMetricsServlet performance improvement

Posted by PengHui Li <pe...@apache.org>.
+1

Great work.

Thanks,
Penghui

On Wed, Feb 23, 2022 at 8:56 PM Enrico Olivelli <eo...@gmail.com> wrote:

> Cool
>
> I also observed such problems but I haven't time to work on a proposal.
>
> Looking forward to see your patch
>
>
> Enrico
>
> Il Mer 23 Feb 2022, 10:43 Jiuming Tao <jm...@streamnative.io.invalid> ha
> scritto:
>
> > Hi all,
> >
> > 1. I have learned that the /metrics endpoint will be requested by more
> than
> > one metrics collect system. In the condition, I want to reimplement
> > `PromethuesMetricsServlet` by sliding window.
> > PrometheusMetricsGenerator#generate will be invoked once in a period(such
> > as 1 minute), the result will be cached and returned for every metrics
> > collect request in the period directly. It could save memory and avoid
> high
> > CPU usage.
> >
> > 2. When there are hundreds MB metrics data collected, it causes high heap
> > memory usage, high CPU usage and GC pressure. In the
> > `PrometheusMetricsGenerator#generate` method, it uses
> > `ByteBufAllocator.DEFAULT.heapBuffer()` to allocate memory for writing
> > metrics data. The default size of `ByteBufAllocator.DEFAULT.heapBuffer()`
> > is 256 bytes, when the buffer resizes, the new buffer capacity is 512
> > bytes(power of 2) and with `mem_copy` operation.
> > If I want to write 100 MB data to the buffer, the current buffer size is
> > 128 MB, and the total memory usage is close to 256 MB (256bytes + 512
> bytes
> > + 1k + .... + 64MB + 128MB). When the buffer size is greater than netty
> > buffer chunkSize(16 MB), it will be allocated as UnpooledHeapByteBuf in
> the
> > heap. After writing metrics data into the buffer, return it to the client
> > by jetty, jetty will copy it into jetty's buffer with memory allocation
> in
> > the heap, again!
> > In this condition, for the purpose of saving memory, avoid high CPU
> > usage(too much memory allocations and `mem_copy` operations) and reducing
> > GC pressure, I want to change `ByteBufAllocator.DEFAULT.heapBuffer()` to
> > `ByteBufAllocator.DEFAULT.compositeDirectBuffer()`, it wouldn't cause
> > `mem_copy` operations and huge memory allocations(CompositeDirectByteBuf
> is
> > a bit slowly in read/write, but it's worth). After writing data, I will
> > call the `HttpOutput#write(ByteBuffer)` method and write it to the
> client,
> > the method won't cause `mem_copy` (I have to wrap ByteBuf to ByteBuffer,
> if
> > ByteBuf wrapped, there will be zero-copy).
> > I tested NO.2 in my local, and it turns out performance is better than
> the
> > heap buffer(below images).
> >
> >
> >
> https://drive.google.com/file/d/1-0drrs9s9kZ2NbbVmzQDwPHdgtpE6QyW/view?usp=sharing
> > (CompositeDirectByteBuf)
> >
> >
> https://drive.google.com/file/d/1-0m15YdsjBudsiweZ4DO7aU3bOFeK17w/view?usp=sharing
> > (PooledHeapByteBuf)
> >
> > Thanks,
> > Tao Jiuming
> >
>

Re: [DISCUSS] PrometheusMetricsServlet performance improvement

Posted by Enrico Olivelli <eo...@gmail.com>.
Cool

I also observed such problems but I haven't time to work on a proposal.

Looking forward to see your patch


Enrico

Il Mer 23 Feb 2022, 10:43 Jiuming Tao <jm...@streamnative.io.invalid> ha
scritto:

> Hi all,
>
> 1. I have learned that the /metrics endpoint will be requested by more than
> one metrics collect system. In the condition, I want to reimplement
> `PromethuesMetricsServlet` by sliding window.
> PrometheusMetricsGenerator#generate will be invoked once in a period(such
> as 1 minute), the result will be cached and returned for every metrics
> collect request in the period directly. It could save memory and avoid high
> CPU usage.
>
> 2. When there are hundreds MB metrics data collected, it causes high heap
> memory usage, high CPU usage and GC pressure. In the
> `PrometheusMetricsGenerator#generate` method, it uses
> `ByteBufAllocator.DEFAULT.heapBuffer()` to allocate memory for writing
> metrics data. The default size of `ByteBufAllocator.DEFAULT.heapBuffer()`
> is 256 bytes, when the buffer resizes, the new buffer capacity is 512
> bytes(power of 2) and with `mem_copy` operation.
> If I want to write 100 MB data to the buffer, the current buffer size is
> 128 MB, and the total memory usage is close to 256 MB (256bytes + 512 bytes
> + 1k + .... + 64MB + 128MB). When the buffer size is greater than netty
> buffer chunkSize(16 MB), it will be allocated as UnpooledHeapByteBuf in the
> heap. After writing metrics data into the buffer, return it to the client
> by jetty, jetty will copy it into jetty's buffer with memory allocation in
> the heap, again!
> In this condition, for the purpose of saving memory, avoid high CPU
> usage(too much memory allocations and `mem_copy` operations) and reducing
> GC pressure, I want to change `ByteBufAllocator.DEFAULT.heapBuffer()` to
> `ByteBufAllocator.DEFAULT.compositeDirectBuffer()`, it wouldn't cause
> `mem_copy` operations and huge memory allocations(CompositeDirectByteBuf is
> a bit slowly in read/write, but it's worth). After writing data, I will
> call the `HttpOutput#write(ByteBuffer)` method and write it to the client,
> the method won't cause `mem_copy` (I have to wrap ByteBuf to ByteBuffer, if
> ByteBuf wrapped, there will be zero-copy).
> I tested NO.2 in my local, and it turns out performance is better than the
> heap buffer(below images).
>
>
> https://drive.google.com/file/d/1-0drrs9s9kZ2NbbVmzQDwPHdgtpE6QyW/view?usp=sharing
> (CompositeDirectByteBuf)
>
> https://drive.google.com/file/d/1-0m15YdsjBudsiweZ4DO7aU3bOFeK17w/view?usp=sharing
> (PooledHeapByteBuf)
>
> Thanks,
> Tao Jiuming
>