Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

MetricManager timers and histograms sample into ExponentiallyDecayingReservoir, whose read lock contends under concurrent store operations

Open Beginner friendly
  • Is something wrong?
#4,988 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
78/100
Issue type
Refactor
Clarity
Clearly specified
Activity status
Active
Tech stack
java
Domain
backend, observability, performance

Research direction

Start in janusgraph-core/src/main/java/org/janusgraph/util/stats/MetricManager.java at getTimer and getHistogram, then inspect MetricInstrumentedStore.runWithMetrics to understand the affected operations. Confirm how MetricRegistry creates these metrics and preserve their names, reporters, and JMX output while using the lock-free reservoir. The change is done when all timers and histograms obtained through MetricManager avoid the contended default reservoir.

Written by the indexing model from the issue text.

Description

  • Version: master (b54c363)
  • Storage Backend: any (the reservoir is in janusgraph-core)
  • Mixed Index Backend: any
  • Expected Behavior: with metrics.enabled=true, recording a duration into stores.getSlice.time, stores.mutate.time or stores.acquireLock.time should cost a few atomic increments, so that instrumentation stays a small fraction of the storage call it measures.
  • Current Behavior: every Timer.update takes the read side of a ReentrantReadWriteLock inside ExponentiallyDecayingReservoir. Under many concurrent transactions the compare-and-set on that lock's state word becomes the hottest code in the process.
Details

MetricManager.getTimer and getHistogram call MetricRegistry.timer(name) / histogram(name):

https://github.com/JanusGraph/janusgraph/blob/b54c363bb/janusgraph-core/src/main/java/org/janusgraph/util/stats/MetricManager.java#L318-L332

Those overloads build the metric with Dropwizard's default reservoir, ExponentiallyDecayingReservoir. Its update method is rescaleIfNeeded(); lockForRegularUsage(); values.put(...); unlockForRegularUsage(); where the regular-usage lock is a ReentrantReadWriteLock.readLock(). Readers do not block each other, but every acquire and every release is a CAS on the lock's single shared state word, plus a ThreadLocal lookup for the per-thread hold count. When many threads update the same timer at once the CASes collide and spin.

MetricInstrumentedStore.runWithMetrics wraps every getSlice, mutate and acquireLock in exactly such a timer, and with the default metrics.merge-stores=true all stores share the same three names, so every backend call in the process funnels into three reservoirs.

A CPU profile of a write-heavy workload on a 128-thread Gremlin Server, taken with async-profiler over 12 minutes, showed:

Frame Flat %
VarHandleInts$FieldInstanceReadWrite.compareAndSet (from AbstractQueuedSynchronizer.compareAndSetState) 17.2
ReentrantReadWriteLock$Sync.tryAcquireShared 9.2
ReentrantReadWriteLock$Sync.tryReleaseShared 9.0
ReentrantReadWriteLock$Sync.fullTryAcquireShared 5.7
AbstractQueuedSynchronizer.getState 3.8
ReentrantReadWriteLock$ReadLock.lock / unlock (self) 4.6
ThreadLocal get/set/remove for the read-hold counter ~4

All of it under com.codahale.metrics.Timer.update → Histogram.update → ExponentiallyDecayingReservoir.update → lockForRegularUsage / unlockForRegularUsage. The storage backend's own calls were under 1% of samples in the same profile. That workload was lock-call heavy, so the share is higher than a typical deployment would see, but the mechanism is the same for any deployment with metrics.enabled=true and enough concurrency.

Suggested Fix

Dropwizard Metrics 4.2 (JanusGraph is on 4.2.37) ships LockFreeExponentiallyDecayingReservoir: the same forward-decay sampling with the same size and alpha defaults, but the reservoir state lives in an immutable object swapped with a single CAS, and update does a putIfAbsent on a ConcurrentSkipListMap with no lock. Its documented trade-off is that a few updates racing with the hourly rescale may be lost, which does not matter for latency percentiles.

MetricRegistry has timer(String, MetricSupplier<Timer>) and histogram(String, MetricSupplier<Histogram>) overloads, so MetricManager can switch every timer and histogram it hands out without changing metric names, reporters or JMX output:

private static final MetricRegistry.MetricSupplier<Timer> TIMER_SUPPLIER =
    () -> new Timer(LockFreeExponentiallyDecayingReservoir.builder().build());

public Timer getTimer(String name) {
    return getRegistry().timer(name, TIMER_SUPPLIER);
}

Every timer and histogram in janusgraph-core, janusgraph-es and the backends goes through MetricManager, so this one change covers them all. Happy to open a PR.

Dominant language
Java
Stars
5.8k
Forks
1.2k
Avg merge
19h 27m
Merged PRs (30d)
21

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from JanusGraph/janusgraph

All issues in JanusGraph/janusgraph

Similar issues

More Java issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.