-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix cacheMissPercentage metric #18208
Changes from 1 commit
1f08bae
bb4a5aa
ba4b9b9
e99322e
cf2957e
2e74aa5
2c42671
2ca2b2f
11dddd6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
package alluxio.worker.block.io; | ||
|
||
import alluxio.metrics.MetricKey; | ||
import alluxio.metrics.MetricsSystem; | ||
|
||
import io.netty.buffer.ByteBuf; | ||
|
||
import java.io.IOException; | ||
import java.nio.ByteBuffer; | ||
import java.nio.channels.ReadableByteChannel; | ||
|
||
/** | ||
* An reader class with metrics. | ||
*/ | ||
public class DeStoreBlockReader extends BlockReader { | ||
private final BlockReader mDeBlockReader; | ||
|
||
/** | ||
* A decorator of BlockReader. | ||
* @param deblockReader block reader | ||
*/ | ||
public DeStoreBlockReader(BlockReader deblockReader) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Requiring the reader to be a Since we are dealing with the bytes-read-cache metric here, requiring the argument to be of type |
||
mDeBlockReader = deblockReader; | ||
} | ||
|
||
@Override | ||
public ByteBuffer read(long offset, long length) throws IOException { | ||
return mDeBlockReader.read(offset, length); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also update the metric here |
||
} | ||
|
||
@Override | ||
public long getLength() { | ||
return mDeBlockReader.getLength(); | ||
} | ||
|
||
@Override | ||
public ReadableByteChannel getChannel() { | ||
return mDeBlockReader.getChannel(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the channel also needs to be wrapped to add the metric updating logic. return new ReadableByteChannel() {
private final ReadableByteChannel mDelegate = mDeBlockReader.getChannel();
@Override
public int read(ByteBuffer dst) throws IOException {
int bytesRead = mDelegate.read(dst);
// update metric here
return bytesRead;
}
} |
||
} | ||
|
||
@Override | ||
public int transferTo(ByteBuf buf) throws IOException { | ||
int bytesReadFromCache = mDeBlockReader.transferTo(buf); | ||
MetricsSystem.counter(MetricKey.WORKER_BYTES_READ_CACHE.getName()).inc(bytesReadFromCache); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
return bytesReadFromCache; | ||
} | ||
|
||
@Override | ||
public boolean isClosed() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also override |
||
return mDeBlockReader.isClosed(); | ||
} | ||
|
||
@Override | ||
public String getLocation() { | ||
return mDeBlockReader.getLocation(); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return mDeBlockReader.toString(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MetricAccountingLocalBlockReader
is a better name.