HenryCaiHaiying commented on code in PR #17025:
URL: https://github.com/apache/iceberg/pull/17025#discussion_r4279164096
##########
kafka-connect/kafka-connect/src/main/java/org/apache/iceberg/connect/channel/Channel.java:
##########
@@ -125,11 +128,16 @@ record -> {
// so increment the record offset by one
controlTopicOffsets.put(record.partition(), record.offset() + 1);
+ long readStart = System.nanoTime();
Event event = AvroUtil.decode(record.value());
+ getChannelMetrics().recordMessageRead((System.nanoTime() -
readStart) / 1_000L);
if (event.groupId().equals(connectGroupId)) {
LOG.debug("Received event of type: {}", event.type().name());
- if (receive(new Envelope(event, record.partition(),
record.offset()))) {
+ long processStart = System.nanoTime();
+ boolean handled = receive(new Envelope(event,
record.partition(), record.offset()));
+ getChannelMetrics().recordMessageProcess((System.nanoTime() -
processStart) / 1_000L);
Review Comment:
I don't see this is a conflict (or double counting) between 2 metrics.
channel-message-process-time is on the outer parent function measuring the time
spent on processing an incoming Kafka message, commit-time is on the child
function measuring the time spent on commit. The parent function can
potentially invoke the child function to process commit, when that happens the
time spent on channel-message-process-time will increase as well. The 2
metrics are measuring their respective measurements, there is no specification
dictating that any 2 time metrics need to be unique and non-overlapping. If
we put in this constraint, any future metrics in a inner function needs to
recursively check all parent functions to see whether there is time metrics on
the outer layer and try to break up the code to make the 2 metrics
non-overlapping in any combination of code paths, we are not going to be able
to maintain the code like that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
