Skip to content

Commit

Permalink
API: Remove counter name (apache#5559)
Browse files Browse the repository at this point in the history
  • Loading branch information
@nastra
nastra authored Aug 18, 2022
1 parent eff2fee commit 1741e4d
Show file tree
Hide file tree
Showing 17 changed files with 121 additions and 265 deletions.
7 changes: 0 additions & 7 deletions api/src/main/java/org/apache/iceberg/metrics/Counter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,6 @@ default Unit unit() {
return Unit.COUNT;
}

/**
* The name of the counter.
*
* @return The name of the counter.
*/
String name();

/**
* Determines whether this counter is a NOOP counter.
*
Expand Down
24 changes: 3 additions & 21 deletions api/src/main/java/org/apache/iceberg/metrics/DefaultCounter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
/** A default {@link Counter} implementation that uses an {@link AtomicLong} to count events. */
public class DefaultCounter implements Counter {
public static final Counter NOOP =
new DefaultCounter("undefined", Unit.UNDEFINED) {
new DefaultCounter(Unit.UNDEFINED) {
@Override
public void increment() {}

Expand All @@ -41,15 +41,12 @@ public long value() {
};

private final LongAdder counter;
private final String name;
private final MetricsContext.Unit unit;
private AsIntCounter asIntCounter = null;
private AsLongCounter asLongCounter = null;

DefaultCounter(String name, MetricsContext.Unit unit) {
Preconditions.checkArgument(null != name, "Invalid counter name: null");
DefaultCounter(MetricsContext.Unit unit) {
Preconditions.checkArgument(null != unit, "Invalid count unit: null");
this.name = name;
this.unit = unit;
this.counter = new LongAdder();
}
Expand All @@ -72,19 +69,14 @@ public long value() {

@Override
public String toString() {
return String.format("%s{%s=%s}", name(), unit().displayName(), value());
return String.format("{%s=%s}", unit().displayName(), value());
}

@Override
public MetricsContext.Unit unit() {
return unit;
}

@Override
public String name() {
return name;
}

MetricsContext.Counter<Integer> asIntCounter() {
if (null == asIntCounter) {
this.asIntCounter = new AsIntCounter();
Expand Down Expand Up @@ -128,11 +120,6 @@ public Integer value() {
public MetricsContext.Unit unit() {
return unit;
}

@Override
public String name() {
return name;
}
}

private class AsLongCounter implements MetricsContext.Counter<Long> {
Expand Down Expand Up @@ -161,10 +148,5 @@ public Long value() {
public MetricsContext.Unit unit() {
return unit;
}

@Override
public String name() {
return name;
}
}
}
8 changes: 4 additions & 4 deletions api/src/main/java/org/apache/iceberg/metrics/DefaultMetricsContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,23 +28,23 @@ public class DefaultMetricsContext implements MetricsContext {
@SuppressWarnings("unchecked")
public <T extends Number> Counter<T> counter(String name, Class<T> type, Unit unit) {
if (Integer.class.equals(type)) {
return (Counter<T>) new DefaultCounter(name, unit).asIntCounter();
return (Counter<T>) new DefaultCounter(unit).asIntCounter();
}

if (Long.class.equals(type)) {
return (Counter<T>) new DefaultCounter(name, unit).asLongCounter();
return (Counter<T>) new DefaultCounter(unit).asLongCounter();
}
throw new IllegalArgumentException(
String.format("Counter for type %s is not supported", type.getName()));
}

@Override
public Timer timer(String name, TimeUnit unit) {
return new DefaultTimer(name, unit);
return new DefaultTimer(unit);
}

@Override
public org.apache.iceberg.metrics.Counter counter(String name, Unit unit) {
return new DefaultCounter(name, unit);
return new DefaultCounter(unit);
}
}
12 changes: 2 additions & 10 deletions api/src/main/java/org/apache/iceberg/metrics/DefaultTimer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,9 @@ public class DefaultTimer implements Timer {
private final TimeUnit timeUnit;
private final LongAdder count = new LongAdder();
private final LongAdder totalTime = new LongAdder();
private final String name;

public DefaultTimer(String name, TimeUnit timeUnit) {
Preconditions.checkArgument(null != name, "Invalid timer name: null");
public DefaultTimer(TimeUnit timeUnit) {
Preconditions.checkArgument(null != timeUnit, "Invalid time unit: null");
this.name = name;
this.timeUnit = timeUnit;
}

Expand Down Expand Up @@ -97,19 +94,14 @@ public void time(Runnable runnable) {
}
}

@Override
public String name() {
return name;
}

@Override
public TimeUnit unit() {
return timeUnit;
}

@Override
public String toString() {
return MoreObjects.toStringHelper(name())
return MoreObjects.toStringHelper(DefaultTimer.class)
.add("duration", totalDuration())
.add("count", count)
.add("timeUnit", timeUnit)
Expand Down
9 changes: 0 additions & 9 deletions api/src/main/java/org/apache/iceberg/metrics/MetricsContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,6 @@ default T value() {
default Unit unit() {
return Unit.UNDEFINED;
}

/**
* The name of the counter.
*
* @return The name of the counter.
*/
default String name() {
return "undefined";
}
}

/**
Expand Down
71 changes: 27 additions & 44 deletions api/src/main/java/org/apache/iceberg/metrics/ScanReport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ public ScanReport build() {

/** A serializable version of a {@link Timer} that carries its result. */
public static class TimerResult {
private final String name;
private final TimeUnit timeUnit;
private final Duration totalDuration;
private final long count;
Expand All @@ -147,23 +146,17 @@ static TimerResult fromTimer(Timer timer) {
if (Timer.NOOP.equals(timer)) {
return null;
}
return new TimerResult(timer.name(), timer.unit(), timer.totalDuration(), timer.count());
return new TimerResult(timer.unit(), timer.totalDuration(), timer.count());
}

TimerResult(String name, TimeUnit timeUnit, Duration totalDuration, long count) {
Preconditions.checkArgument(null != name, "Invalid timer name: null");
TimerResult(TimeUnit timeUnit, Duration totalDuration, long count) {
Preconditions.checkArgument(null != timeUnit, "Invalid time unit: null");
Preconditions.checkArgument(null != totalDuration, "Invalid duration: null");
this.name = name;
this.timeUnit = timeUnit;
this.totalDuration = totalDuration;
this.count = count;
}

public String name() {
return name;
}

public TimeUnit timeUnit() {
return timeUnit;
}
Expand All @@ -178,7 +171,7 @@ public long count() {

@Override
public String toString() {
return MoreObjects.toStringHelper(name())
return MoreObjects.toStringHelper(TimerResult.class)
.add("duration", totalDuration())
.add("count", count)
.add("timeUnit", timeUnit)
Expand All @@ -197,20 +190,18 @@ public boolean equals(Object o) {

TimerResult that = (TimerResult) o;
return count == that.count
&& Objects.equal(name, that.name)
&& timeUnit == that.timeUnit
&& Objects.equal(totalDuration, that.totalDuration);
}

@Override
public int hashCode() {
return Objects.hashCode(name, timeUnit, totalDuration, count);
return Objects.hashCode(timeUnit, totalDuration, count);
}
}

/** A serializable version of a {@link Counter} that carries its result. */
public static class CounterResult {
private final String name;
private final MetricsContext.Unit unit;
private final long value;

Expand All @@ -219,21 +210,15 @@ static CounterResult fromCounter(Counter counter) {
if (counter.isNoop()) {
return null;
}
return new CounterResult(counter.name(), counter.unit(), counter.value());
return new CounterResult(counter.unit(), counter.value());
}

CounterResult(String name, Unit unit, long value) {
Preconditions.checkArgument(null != name, "Invalid counter name: null");
CounterResult(Unit unit, long value) {
Preconditions.checkArgument(null != unit, "Invalid counter unit: null");
this.name = name;
this.unit = unit;
this.value = value;
}

public String name() {
return name;
}

public Unit unit() {
return unit;
}
Expand All @@ -244,7 +229,7 @@ public long value() {

@Override
public String toString() {
return String.format("%s{%s=%s}", name(), unit().displayName(), value());
return String.format("{%s=%s}", unit().displayName(), value());
}

@Override
Expand All @@ -258,14 +243,12 @@ public boolean equals(Object o) {
}

CounterResult that = (CounterResult) o;
return Objects.equal(name, that.name)
&& unit == that.unit
&& Objects.equal(value, that.value);
return unit == that.unit && Objects.equal(value, that.value);
}

@Override
public int hashCode() {
return Objects.hashCode(name, unit, value);
return Objects.hashCode(unit, value);
}
}

Expand Down Expand Up @@ -355,15 +338,15 @@ public CounterResult totalDeleteFileSizeInBytes() {
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.addValue(totalPlanningDuration)
.addValue(resultDataFiles)
.addValue(resultDeleteFiles)
.addValue(scannedDataManifests)
.addValue(skippedDataManifests)
.addValue(totalDataManifests)
.addValue(totalDeleteManifests)
.addValue(totalFileSizeInBytes)
.addValue(totalDeleteFileSizeInBytes)
.add("totalPlanningDuration", totalPlanningDuration)
.add("resultDataFiles", resultDataFiles)
.add("resultDeleteFiles", resultDeleteFiles)
.add("totalDataManifests", totalDataManifests)
.add("totalDeleteManifests", totalDeleteManifests)
.add("scannedDataManifests", scannedDataManifests)
.add("skippedDataManifests", skippedDataManifests)
.add("totalFileSizeInBytes", totalFileSizeInBytes)
.add("totalDeleteFileSizeInBytes", totalDeleteFileSizeInBytes)
.toString();
}

Expand Down Expand Up @@ -486,15 +469,15 @@ public Counter skippedDataManifests() {
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.addValue(totalPlanningDuration)
.addValue(resultDataFiles)
.addValue(resultDeleteFiles)
.addValue(scannedDataManifests)
.addValue(skippedDataManifests)
.addValue(totalDataManifests)
.addValue(totalDeleteManifests)
.addValue(totalFileSizeInBytes)
.addValue(totalDeleteFileSizeInBytes)
.add("totalPlanningDuration", totalPlanningDuration)
.add("resultDataFiles", resultDataFiles)
.add("resultDeleteFiles", resultDeleteFiles)
.add("totalDataManifests", totalDataManifests)
.add("totalDeleteManifests", totalDeleteManifests)
.add("scannedDataManifests", scannedDataManifests)
.add("skippedDataManifests", skippedDataManifests)
.add("totalFileSizeInBytes", totalFileSizeInBytes)
.add("totalDeleteFileSizeInBytes", totalDeleteFileSizeInBytes)
.toString();
}
}
Expand Down
27 changes: 16 additions & 11 deletions api/src/main/java/org/apache/iceberg/metrics/Timer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,6 @@ public interface Timer {
*/
Timed start();

/**
* The name of the timer.
*
* @return The name of the timer.
*/
default String name() {
return "undefined";
}

/**
* The {@link TimeUnit} of the timer.
*
Expand Down Expand Up @@ -112,6 +103,15 @@ default void time(Duration duration) {
*/
<T> T time(Supplier<T> supplier);

/**
* Determines whether this timer is a NOOP timer.
*
* @return Whether this timer is a NOOP timer.
*/
default boolean isNoop() {
return NOOP.equals(this);
}

/**
* A timing sample that carries internal state about the Timer's start position. The timing can be
* completed by calling {@link Timed#stop()}.
Expand All @@ -137,12 +137,17 @@ public Timed start() {

@Override
public long count() {
return 0;
throw new UnsupportedOperationException("NOOP timer has no count");
}

@Override
public Duration totalDuration() {
return Duration.ZERO;
throw new UnsupportedOperationException("NOOP timer has no duration");
}

@Override
public TimeUnit unit() {
throw new UnsupportedOperationException("NOOP timer has no unit");
}

@Override
Expand Down
Loading

0 comments on commit 1741e4d

Please sign in to comment.