KranzL commented on PR #2039: URL: https://github.com/apache/iceberg-go/pull/2039#issuecomment-5785862110
149c395 covers the failure path and the rest of the review: - fail is the only error path into records and it now hands the worker's credit to the error record when no Last record was sent, so no token is abandoned on any path; TestRecordSinkHandsOffCreditOnce pins that. - Scan.ReadTasks, ToArrowRecords and WithMaxConcurrency document the bound for the general scan and the size-skew case. - TestExecuteCompactionGroupRecordPipelineBounded checks both terms of the WithCompactionArrowBatchSize formula through ExecuteCompactionGroup and the clustered writer. - The gate stays for numWorkers == 1; the concurrency-1 benchmark is in that thread. On the 100 ms idle timer: it only decides when the benchmark opens the gate, and gated-batches is reported next to peak-heap-delta-MB, so an early open shows up as a count below M - 1 before the change or below workers - 1 after it. Every row in the description reports the full count. Fresh numbers, same machine and method as the description, interleaved binaries for de53d44 and 149c395, benchstat n = 10: | sub-benchmark | de53d44, sec/op | 149c395, sec/op | change | |---|---:|---:|---| | extra_properties_0 | 163.1m ± 16% | 165.5m ± 7% | ~ (p=0.579) | | extra_properties_16 | 166.4m ± 6% | 170.7m ± 8% | ~ (p=0.579) | | extra_properties_64 | 165.0m ± 8% | 171.7m ± 10% | ~ (p=0.529) | | geomean | 164.8m | 169.3m | +2.70% | B/op, allocs/op, batches/op and files/op are unchanged. These runs were on battery power, so the absolute times are about 2.2x the ones in the description; both binaries ran under the same conditions, interleaved. BenchmarkArrowScanReorderHeapLaggingTask on 149c395, -count=5: gated-batches is 3 in all 15 runs; peak-heap-delta-MB is 2.07, 2.10, 2.09, 2.10, 2.11 at 8 files, 2.20, 2.27, 2.13, 2.17, 2.16 at 32 files and 2.34, 2.27, 2.23, 2.24, 2.28 at 128 files. go test ./table/... -race -count=1 passes and golangci-lint with a clean cache reports no issues for ./table/.... -- 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]
