tjiuming opened a new pull request, #17852:
URL: https://github.com/apache/pulsar/pull/17852

   ### Motivation
   
   This PR is fully based on https://github.com/apache/pulsar/pull/17618, but 
the origin PR has a memory leak issue, so it was reverted.
   This PR is supposed to fix the memory leak issue.
   
   Why the origin PR has a memory leak issue:
   In https://github.com/apache/pulsar/pull/17618, 
PrometheusMetricsGenerator.java:
   ```
       public static void generate(PulsarService pulsar, boolean 
includeTopicMetrics, boolean includeConsumerMetrics,
                                   boolean includeProducerMetrics, boolean 
splitTopicAndPartitionIndexLabel,
                                   OutputStream out,
                                   List<PrometheusRawMetricsProvider> 
metricsProviders)
               throws IOException {
           ByteBuf buf = ByteBufAllocator.DEFAULT.heapBuffer();
           boolean exceptionHappens = false;
           //Used in namespace/topic and transaction aggregators as share 
metric names
           PrometheusMetricStreams metricStreams = new 
PrometheusMetricStreams();
           try {
               SimpleTextOutputStream stream = new SimpleTextOutputStream(buf);
   
               generateSystemMetrics(stream, 
pulsar.getConfiguration().getClusterName());
   
               NamespaceStatsAggregator.generate(pulsar, includeTopicMetrics, 
includeConsumerMetrics,
                       includeProducerMetrics, 
splitTopicAndPartitionIndexLabel, metricStreams);
   
               if (pulsar.getWorkerServiceOpt().isPresent()) {
                   pulsar.getWorkerService().generateFunctionsStats(stream);
               }
   
               if (pulsar.getConfiguration().isTransactionCoordinatorEnabled()) 
{
                   TransactionAggregator.generate(pulsar, metricStreams, 
includeTopicMetrics);
               }
   
               metricStreams.flushAllToStream(stream);
   
               generateBrokerBasicMetrics(pulsar, stream);
   
               generateManagedLedgerBookieClientMetrics(pulsar, stream);
   
               if (metricsProviders != null) {
                   for (PrometheusRawMetricsProvider metricsProvider : 
metricsProviders) {
                       metricsProvider.generate(stream);
                   }
               }
               out.write(buf.array(), buf.arrayOffset(), buf.readableBytes());
           } finally {
               //release all the metrics buffers
               metricStreams.releaseAll();
               //if exception happens, release buffer
               if (exceptionHappens) {
                   buf.release();
               }
           }
       }
   ```
   in the finally scope, call `buf.release()` when `exceptionHappens == true`. 
But the initialize value of `exceptionHappens` is false, and it never updated 
to true anywhere. So the memory leak issue happens.
   It should be a small mistake in the check pick process.
   
   ### Documentation
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   - [ ] `doc-required` 
   (Your PR needs to update docs and you will update later)
   
   - [x] `doc-not-needed` 
   (Please explain why)
   
   - [ ] `doc` 
   (Your PR contains doc changes)
   
   - [ ] `doc-complete`
   (Docs have been already added)
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to