Skip to content

Commit

Permalink
metrics: remove delayed implementation of timers interface (elastic#1…
Browse files Browse the repository at this point in the history
…4815)

The delayed implementation `AfterCompletionTimerMetric` of the `TimerMetric`
interface, introduced along-side that interface's introduction to replicate
the previous (undesired) behaviour, is superceded by an already-merged live-
tracking implementation that is effectively as performant when not under
concurrent contention and still reasonably performant when a single timer is
contended across multiple threads.

The `metric.timers` setting removed here has not been a part of any Logstash
release and can safely be removed without going through the normal deprecation
path; from the user's perspective this removal combined with the previously-
merged work is simply an improvement to the accuracy of the existing timer
metrics exposed via our API.
  • Loading branch information
yaauie authored Jan 12, 2023
1 parent 6bbe070 commit 42ce9fc
Show file tree
Hide file tree
Showing 6 changed files with 1 addition and 84 deletions.
1 change: 0 additions & 1 deletion docker/data/logstash/env2yaml/env2yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ func normalizeSetting(setting string) (string, error) {
"log.format",
"modules",
"metric.collect",
"metric.timers",
"path.logs",
"path.plugins",
"api.auth.type",
Expand Down
2 changes: 0 additions & 2 deletions logstash-core/lib/logstash/environment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ module Environment
Setting::Boolean.new("config.support_escapes", false),
Setting::String.new("config.field_reference.escape_style", "none", true, %w(none percent ampersand)),
Setting::Boolean.new("metric.collect", true),
Setting::String.new("metric.timers", "delayed", true, %w(delayed live)),
Setting::String.new("pipeline.id", "main"),
Setting::Boolean.new("pipeline.system", false),
Setting::PositiveInteger.new("pipeline.workers", LogStash::Config::CpuCoreStrategy.maximum),
Expand Down Expand Up @@ -128,7 +127,6 @@ module Environment
java.lang.System.setProperty("ls.log.format", settings.get("log.format"))
java.lang.System.setProperty("ls.log.level", settings.get("log.level"))
java.lang.System.setProperty("ls.pipeline.separate_logs", settings.get("pipeline.separate_logs").to_s)
java.lang.System.setProperty("ls.metric.timers", settings.get("metric.timers"))
unless java.lang.System.getProperty("log4j.configurationFile")
log4j_config_location = ::File.join(settings.get("path.settings"), "log4j2.properties")

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,6 @@ public TimerMetric create(final String name) {
}

TimerMetric create(final String name, final LongSupplier nanoTimeSupplier) {
// INTERNAL-ONLY system property escape hatch, set with `metric.timers` config in logstash.yml
final String timerType = System.getProperty("ls.metric.timers", "delayed");
switch (timerType) {
case "live" : return new ConcurrentLiveTimerMetric(name, nanoTimeSupplier);
case "delayed": return new AfterCompletionTimerMetric(name, nanoTimeSupplier);
default : throw new IllegalStateException(String.format("Unknown timer type `%s`", timerType));
}
return new ConcurrentLiveTimerMetric(name, nanoTimeSupplier);
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@ public TestTimerMetricFactory(final LongSupplier nanoTimeSupplier) {
this.nanoTimeSupplier = nanoTimeSupplier;
}

public AfterCompletionTimerMetric newAfterCompletionTimerMetric(final String name) {
return new AfterCompletionTimerMetric(name, this.nanoTimeSupplier);
}

public ConcurrentLiveTimerMetric newConcurrentLiveTimerMetric(final String name) {
return new ConcurrentLiveTimerMetric(name, this.nanoTimeSupplier);
}
Expand Down

0 comments on commit 42ce9fc

Please sign in to comment.