andygrove commented on issue #6399: URL: https://github.com/apache/datafusion-comet/issues/6399#issuecomment-5895791465
Phase 2 is done. Sonnet agents triaged all 292 PRs that touch production code, in 25 batches. Each PR got HIGH, MEDIUM or LOW for how plausibly its diff regressed something that worked in 1.0.0. The result is 47 HIGH, 119 MEDIUM and 126 LOW. A HIGH is a reason to look closer, not a finding, and I expect most of them to turn out fine in Phase 3. The method changed in two ways from the plan above. First, the packets left out later fixes to each PR's lines, because including them would have given away the seven known answers from Phase 1. Second, one agent looked up my local notes, which describe some of those answers, so the later batches were told to judge each PR only from its own diff and the code. For calibration, five of the seven PRs behind the Phase 1 regressions were rated HIGH. Two of those five aren't clean hits: #5368 shared a batch with its own fix, and the agent that rated #5602 had read my notes. Of the five blind checks, #4775, #4870 and #5262 were rated HIGH, and #6041 and #5314 were rated MEDIUM. Both misses only showed up under a non-default configuration: - `spark.sql.codegen.factoryMode=NO_CODEGEN` on Spark 3.4 - a `fs.comet.libhdfs.schemes` list naming `s3` but not `s3a` Both were at least flagged as touching a default path. That's within the plan's tolerance of one or two misses, so I'm not rerunning the triage. The 47 HIGH PRs go to Phase 3, grouped by area: - Expressions and casts (12): #4775 #4816 #5166 #5280 #5415 #5452 #5472 #5614 #5638 #5682 #5692 #5773 - Planner, operators and shuffle (13): #4870 #5041 #5362 #5421 #5442 #5531 #5615 #5667 #5668 #5723 #5763 #5803 #5916 - Scans, meaning Parquet, Iceberg and Delta (15): #5177 #5237 #5262 #5377 #5503 #5602 #5654 #5681 #5715 #5732 #5786 #5853 #6116 #6154 #6219 - Memory, FFI, config and shims (7): #5368 #5493 #5539 #5552 #6025 #6166 #6191 The dependency-upgrade review in Phase 3 starts from #5262, #5324, #5865 and #6094. The common reasons for a HIGH were: - An expression or operator that became native by default, with a Spark difference that the tests don't pin down (NaN and -0.0, regex dialects, patch-version behavior). - A rewrite or "no behavior change" refactor of a default-path kernel with no new correctness tests. - A default-path change bundled into a PR for an opt-in feature. For example, #5668 made partitions compute eagerly on every native shuffle read, and #5531 added a new hard error on the local shuffle read path. - A logged fallback turned into a thrown error, as in #6154. One open question goes first in Phase 3 because it could affect the release. #6261 fixed a hang that needs a single Tokio worker, and it isn't on `branch-1.1`. A standalone executor without `spark.executor.cores` gets exactly one worker. Phase 1 traced the memory-release bug in #6261 to before 1.0.0, but nobody has run the hang reproducer on 1.0.0. So it isn't known yet whether the hang is new in 1.1.0. -- 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]
