zeroshade opened a new pull request, #938:
URL: https://github.com/apache/arrow-go/pull/938

   ### What / Why
   
   The **Benchmarks** workflow on `main` has been failing intermittently. Root 
cause is two layers:
   
   1. `ci/scripts/bench.sh` runs `go test -bench=. ... | tee bench_stat.dat` 
under `set -ex` (no `pipefail`), so the pipeline exit code is `tee`'s. 
Benchmark panics/`FAIL`s were **swallowed**, producing false-success runs and 
only failing randomly when `gobenchdata` couldn't parse the panic-corrupted 
output or the Conbench upload timed out.
   2. Several benchmarks were actually broken (hidden by the above).
   
   ### Fixes
   
   **Broken benchmarks / code**
   - **`parquet/metadata`** — `checkBulk` was declared but never assigned in 
any build variant, so the exported `blockSplitBloomFilter.CheckBulk` always 
panicked on a nil func pointer. Added `checkBulkGo` and wired it up in both the 
`default` and `amd64` init paths, plus a regression test 
(`TestBlockSplitBloomFilterCheckBulk`).
   - **`arrow/array`** — `BenchmarkBinaryBuilder_LargeData` / `_WithReserve` 
passed a `nil` datatype to `NewBinaryBuilder`, panicking at construction. Now 
use `arrow.BinaryTypes.Binary`.
   - **`parquet/internal/encoding`** — `BenchmarkMemoTableHighCardinality` 
asserted an exact unique count but generated values from a randomly generated 
dictionary, which does not guarantee uniqueness. Now builds exactly `nunique` 
distinct values.
   - **`arrow/compute`** — `BenchmarkRunEndKernels` sized `String` inputs at 
the L3 cache size; on large-L3 runners this overflowed int32 value offsets and 
panicked with a negative slice bound. The String element count is now capped so 
a cache-sized run stays within int32 offset limits.
   
   **Pipeline hardening**
   - `ci/scripts/bench.sh`: `set -exo pipefail` so benchmark failures fail the 
job instead of being masked.
   - `.github/workflows/benchmark.yml`: pin the `benchadapt` install to a fixed 
`conbench` commit for reproducibility.
   
   ### Testing
   
   Locally, all previously-failing benchmarks now pass (`-benchtime=1x`):
   - `go test -run=^$ -bench=BenchmarkBinaryBuilder_LargeData ./arrow/array/`
   - `go test -run=TestBlockSplitBloomFilterCheckBulk ./parquet/metadata/` and 
`-bench=BenchmarkFilterCheckBulk`
   - `go test -run=^$ -bench=BenchmarkMemoTableHighCardinality 
./parquet/internal/encoding/`
   - `go test -run=^$ -bench='BenchmarkRunEndKernels/.../value_type=utf8' 
./arrow/compute/`
   
   `gofmt` and `go vet` are clean on the changed files.
   
   ### Note (out of scope)
   
   Older failures also showed Conbench upload timeouts (`504`/`408` from 
`conbench.arrow-dev.org`) — a server-side issue independent of this change. If 
desired, a follow-up could make the upload step best-effort (non-blocking) so 
external outages don't fail the build.
   


-- 
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]

Reply via email to