malinjawi commented on PR #12592: URL: https://github.com/apache/gluten/pull/12592#issuecomment-6321589277
> @malinjawi I don't think the `xxx.empty` marker really helps. It doesn't make dumping work with `buildHashTableOncePerExecutor.enabled=true`, and the micro benchmark can't read the file anyway. > > As I understand it, this PR is addressing two different issues: > > 1. A stage with an empty input iterator crashes because of the null `writer_`. The null check fixes that. But now it leaves a `xxx.empty` file that the micro benchmark can't read, so the stage still can't be replayed. Empty input iterators can be a real case, so I'd suggest making the micro benchmark treat the marker as an empty input iterator. Then `--data` positions still line up. > 2. Dumping doesn't work with `buildHashTableOncePerExecutor.enabled=true`, but does with `=false`. Instead of writing a useless `xxx.empty` marker file, I'd suggest either force `buildHashTableOncePerExecutor.enabled=false` when dumping is enabled and log a warning, or throw an exception for that configuration and document it. In both cases the `xxx.empty` file isn't needed for the BHJ case, and users don't have to learn about it from the docs. @marin-ma Thanks, makes sense. Updated: 1. The micro benchmark now reads a `.empty` marker as an empty input iterator, so `--data` positions line up. 2. When dumping is enabled, `buildHashTableOncePerExecutor.enabled` is forced off with a warning, so the BHJ build side gets dumped as a normal data file. Also the input iterators are only dumped once the native side reads them, and a BHJ using the cached hash table never reads its build side. So that iterator wasn't dumped at all, not even as a marker. With the build now done per task, a BHJ stage dumps and replays end to end. -- 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]
