velox icon indicating copy to clipboard operation
velox copied to clipboard

Fix probe spill check failure

Open xiaoxmeng opened this issue 10 months ago • 3 comments

The maybeReadSpillOutput has a bug that reset spillInputReader_ but not spillOutputReader_ during spill output processing which caused the wrong probe finish condition to be detected. The hash probe operator finishes early and tries to restore the same spill partition that run into spill partition restore check failure. Add unit test to reproduce and verify the fix.

@zhztheplayer also helped verify the fix on Gluten CI.

xiaoxmeng avatar Apr 02 '24 06:04 xiaoxmeng

Deploy Preview for meta-velox canceled.

Name Link
Latest commit f2bc5c04dc1489c3b7249452d51239db764c9787
Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/660ef4b66cf61400088e24c6

netlify[bot] avatar Apr 02 '24 06:04 netlify[bot]

@xiaoxmeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Apr 02 '24 07:04 facebook-github-bot

@xiaoxmeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Apr 04 '24 07:04 facebook-github-bot

This pull request was exported from Phabricator. Differential Revision: D55627788

facebook-github-bot avatar Apr 04 '24 18:04 facebook-github-bot

@xiaoxmeng merged this pull request in facebookincubator/velox@08dbd0a673533f6bcdfb47394573b40e9bb14e48.

facebook-github-bot avatar Apr 04 '24 20:04 facebook-github-bot

Conbench analyzed the 1 benchmark run on commit 08dbd0a6.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

conbench-facebook[bot] avatar Apr 04 '24 21:04 conbench-facebook[bot]