Copilot commented on code in PR #10096:
URL: https://github.com/apache/seatunnel/pull/10096#discussion_r2569225737
##########
seatunnel-engine/seatunnel-engine-ui/src/views/jobs/detail.tsx:
##########
@@ -94,26 +98,26 @@ export default defineComponent({
const sourceCell = (
row: Vertex,
key:
- | 'TableSourceReceivedBytes'
- | 'TableSourceReceivedCount'
- | 'TableSourceReceivedQPS'
- | 'TableSourceReceivedBytesPerSeconds'
+ | 'SourceReceivedBytes'
+ | 'SourceReceivedCount'
+ | 'SourceReceivedQPS'
+ | 'SourceReceivedBytesPerSeconds'
) => {
if (row.type === 'source') {
- return row.tablePaths.reduce((s, path) => s +
Number(job.metrics?.[key][path]), 0)
+ return row.tablePaths.reduce((s, path) => s +
Number(job.metrics?.[key]), 0)
}
return 0
}
const sinkCell = (
row: Vertex,
key:
- | 'TableSinkWriteBytes'
- | 'TableSinkWriteCount'
- | 'TableSinkWriteQPS'
- | 'TableSinkWriteBytesPerSeconds'
+ | 'SinkWriteBytes'
+ | 'SinkWriteCount'
+ | 'SinkWriteQPS'
+ | 'SinkWriteBytesPerSeconds'
) => {
if (row.type === 'sink') {
- return row.tablePaths.reduce((s, path) => s +
Number(job.metrics?.[key][path]), 0)
+ return row.tablePaths.reduce((s, path) => s +
Number(job.metrics?.[key]), 0)
Review Comment:
The `reduce` operation no longer makes sense after removing the `[path]`
indexing. The code now accesses an aggregate metric value
(`job.metrics?.[key]`) which is a single string, not a path-indexed object.
This means the same value will be added multiple times (once for each path in
`tablePaths`), multiplying the actual metric value by the number of paths.
Either:
1. Keep using the path-specific metric keys (`TableSinkWriteBytes` etc.)
with `[path]` indexing, OR
2. Remove the reduce and just return `Number(job.metrics?.[key])` directly
when accessing aggregate metrics.
The current implementation will produce incorrect (inflated) values.
##########
seatunnel-engine/seatunnel-engine-ui/src/views/jobs/detail.tsx:
##########
@@ -94,26 +98,26 @@ export default defineComponent({
const sourceCell = (
row: Vertex,
key:
- | 'TableSourceReceivedBytes'
- | 'TableSourceReceivedCount'
- | 'TableSourceReceivedQPS'
- | 'TableSourceReceivedBytesPerSeconds'
+ | 'SourceReceivedBytes'
+ | 'SourceReceivedCount'
+ | 'SourceReceivedQPS'
+ | 'SourceReceivedBytesPerSeconds'
) => {
if (row.type === 'source') {
- return row.tablePaths.reduce((s, path) => s +
Number(job.metrics?.[key][path]), 0)
+ return row.tablePaths.reduce((s, path) => s +
Number(job.metrics?.[key]), 0)
Review Comment:
The `reduce` operation no longer makes sense after removing the `[path]`
indexing. The code now accesses an aggregate metric value
(`job.metrics?.[key]`) which is a single string, not a path-indexed object.
This means the same value will be added multiple times (once for each path in
`tablePaths`), multiplying the actual metric value by the number of paths.
Either:
1. Keep using the path-specific metric keys (`TableSourceReceivedBytes`
etc.) with `[path]` indexing, OR
2. Remove the reduce and just return `Number(job.metrics?.[key])` directly
when accessing aggregate metrics.
The current implementation will produce incorrect (inflated) values.
--
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]
