A question about MetricsPersistentWorker context map

[andyzzl]

Please answer these questions before submitting your issue.

• Why do you submit this issue?
• Question or discussion
• Bug
• Requirement
• Feature or performance improvement

---

Question

• What do you want to know?

Hi, I have a question about the code of MetricsPersistentWorker.

private void flushDataToStorage(List<Metrics> metricsList,
                                    List<PrepareRequest> prepareRequests) {
        try {
            loadFromStorage(metricsList);

            for (Metrics metrics : metricsList) {
                Metrics cachedMetrics = context.get(metrics);
                if (cachedMetrics != null) {
                    /*
                     * If the metrics is not supportUpdate, defined through MetricsExtension#supportUpdate,
                     * then no merge and further process happens.
                     */
                    if (!supportUpdate) {
                        continue;
                    }
                    /*
                     * Merge metrics into cachedMetrics, change only happens inside cachedMetrics.
                     */
                    final boolean isAbandoned = !cachedMetrics.combine(metrics);
                    if (isAbandoned) {
                        continue;
                    }
                    cachedMetrics.calculate();
                    prepareRequests.add(metricsDAO.prepareBatchUpdate(model, cachedMetrics));
                    nextWorker(cachedMetrics);
                } else {
                    metrics.calculate();
                    prepareRequests.add(metricsDAO.prepareBatchInsert(model, metrics));
                    nextWorker(metrics);
                }

                /*
                 * The `metrics` should be not changed in all above process. Exporter is an async process.
                 */
                nextExportWorker.ifPresent(exportEvenWorker -> exportEvenWorker.in(
                    new ExportEvent(metrics, ExportEvent.EventType.INCREMENT)));
            }
        } catch (Throwable t) {
            log.error(t.getMessage(), t);
        } finally {
            metricsList.clear();
        }
    }

In the function flushDataToStorage, a metrics object which not stored in context is considered as a new metrics, otherwise, it needs to be combined with the old metrics in context. After prepareRequests of a new metrics generated, we don't put this new metrics into context. So in the next run of PersistenceTimer, when a metrics with same id comes, we have to query db again. Is there some problems cause that we can't put a new metrics object into context?

Thanks.

[wu-sheng]

a metrics object which not stored in context is considered as a new metrics, otherwise, it needs to be combined with the old metrics in context

This is not a correct conclusion. Read #loadFromStorage, context#put is called if metrics could be loaded from database.

[andyzzl]

Sorry I didn't make it clear. I mean after loadFromStorage called, if a metrics can't be found in context still, it is a new metrics.

[wu-sheng]

Sorry I didn't make it clear. I mean after loadFromStorage called, if a metrics can't be found in context still, it is a new metrics.

Why it isn't? It means you need to insert it into the database, rather than updating.

[andyzzl]

After prepareRequests generated, we can put it into context, both db and context are updated. So we don’t need to query db in next run.

[wu-sheng]

That depends on the context. I deleted a comment about this.

Thinking about this, if your service includes parameterized URI, such as /prod/{id}/create, but you don't always have grouping rules, or SpringMVC(even you have, there is Gateway before your Spring controller codes), then every request could create different endpoints. All these endpoints have one access but cost memory for nearly 1 minute.(context only has an expired period).
Considering this, do you still want to add this logic?

[andyzzl]

Oh, I never thought of this situation. It do cost too much memory. Thank you for your explanation.