You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "crepererum (via GitHub)" <gi...@apache.org> on 2023/09/11 16:44:41 UTC

[GitHub] [arrow-datafusion] crepererum opened a new issue, #7522: Expensive backtraces

crepererum opened a new issue, #7522:
URL: https://github.com/apache/arrow-datafusion/issues/7522

   DataFusion creates loads of errors even on the happy path. However as of #7434, we now gather a backtrace for each error. This is rather expensive. Here is a profile dump from a prod workload:
   
   ![Screenshot from 2023-09-11 18-35-55](https://github.com/apache/arrow-datafusion/assets/1529400/c6541616-18dd-44a3-b0ce-2f82500b1b9f)
   
   In the said workload, there is a LOT of going for bookkeeping (that's why I had the profiler running in the first place) but the backtraces alone make up for 30% of the time. The place looks like this:
   
   ```text
   logical:
   Projection: ...
     TableScan: ...                                                                                                                                                                                                                                                                                                                                                                                               
   
   physical:
   ProjectionExec: ..
     CoalesceBatchesExec: target_batch_size=8192
       FilterExec: ...
         ParquetExec: ...
   ```
   
   (had to remove a good amount of details due to data protection, but the filters / predicates are rather simple)
   
   I think there are two paths forward:
   
   - do NOT generate backtraces for errors
   - do NOT use errors for the happy path but rather `Option` or some other enum


-- 
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: github-unsubscribe@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] alamb commented on issue #7522: Performance Regression: Backtraces in errors slow down planning time (Expensive backtraces)

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7522:
URL: https://github.com/apache/arrow-datafusion/issues/7522#issuecomment-1718371013

   I looked into why Errors were being created so frequently and I found and filed https://github.com/apache/arrow-datafusion/issues/7553 
   
   I also filed https://github.com/apache/arrow-datafusion/issues/7552 to track the general notion of not using errors in the happy path


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] alamb commented on issue #7522: Performance Regression: Backtraces in errors slow down planning time (Expensive backtraces)

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7522:
URL: https://github.com/apache/arrow-datafusion/issues/7522#issuecomment-1714472794

   > For the happy path, would you mind to clarify a bit the idea?
   
   I think the idea is that there are many places in the code that use fallible functions (on` DFSchema` and otherwise) and then call `ok()` or `is_ok()` and discard the error. This means that in the happy path many `DataFusionError`s are created that are not returned to the user but instead are created during the course of normal query processing


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] comphead commented on issue #7522: Performance Regression: Backtraces in errors slow down planning time (Expensive backtraces)

Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on issue #7522:
URL: https://github.com/apache/arrow-datafusion/issues/7522#issuecomment-1714340006

   @crepererum if you run your workload with `RUST_BACKTRACE=0` do you still experience performance degradation?


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] alamb closed issue #7522: Performance Regression: Backtraces in errors slow down planning time (Expensive backtraces)

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #7522: Performance Regression: Backtraces in errors slow down planning time (Expensive backtraces)
URL: https://github.com/apache/arrow-datafusion/issues/7522


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] alamb commented on issue #7522: Performance Regression: Backtraces in errors slow down planning time (Expensive backtraces)

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7522:
URL: https://github.com/apache/arrow-datafusion/issues/7522#issuecomment-1715795306

   > Also its probably good to have a benchmark test for planner, not sure how to do it though, but its doable. @alamb @crepererum let me know your thoughts
   
   I think there is one benchmark for the planner here:
   
   ```shell
   cargo bench --bench sql_planner
   ```
   
   I haven't run it recently -- it would be great to eventually run these over time and track their performance #5504 


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] crepererum commented on issue #7522: Performance Regression: Backtraces in errors slow down planning time (Expensive backtraces)

Posted by "crepererum (via GitHub)" <gi...@apache.org>.
crepererum commented on issue #7522:
URL: https://github.com/apache/arrow-datafusion/issues/7522#issuecomment-1715228022

   I can check w/ `RUST_BACKTRACE=0` later. However note that we (InfluxData) run our Rust binaries with `RUST_BACKTRACE=1` in prod because otherwise you're pretty helpless in case of unexpected panics. So if you wanna make this env-based, maybe use a different variable instead of hijacking the one that Rust offers?


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] alamb commented on issue #7522: Performance Regression: Backtraces in errors slow down planning time (Expensive backtraces)

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7522:
URL: https://github.com/apache/arrow-datafusion/issues/7522#issuecomment-1714265299

   cc @comphead 
   
   > I think there are two paths forward:
   > do NOT generate backtraces for errors
   > do NOT use errors for the happy path but rather Option or some other enum
   
   I agree with this assessment. What I propose we do is:
   
   1. revert https://github.com/apache/arrow-datafusion/pull/7434 while we sort things out
   
   2. Try and work on avoiding errors on the happy path (https://github.com/apache/arrow-datafusion/issues/5309 is related) which is a somewhat unfortunate pattern (as each error requires at least one string allocation!) among other things


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] comphead commented on issue #7522: Performance Regression: Backtraces in errors slow down planning time (Expensive backtraces)

Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on issue #7522:
URL: https://github.com/apache/arrow-datafusion/issues/7522#issuecomment-1714338169

   Thanks for reporting this. I'm thinking of adding an extra check whether backtrace should be generated.
   
   For the happy path, would you mind to clarify a bit the idea?


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] comphead commented on issue #7522: Performance Regression: Backtraces in errors slow down planning time (Expensive backtraces)

Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on issue #7522:
URL: https://github.com/apache/arrow-datafusion/issues/7522#issuecomment-1714548591

   as a quick fix I can do backtrace even more optional, like crate feature(the same done in anyhow crate) or put another datafusion config param to enable/disable backtrace. 
   
   Also its probably good to have a benchmark test for planner, not sure how to do it though, but its doable. @alamb @crepererum let me know your thoughts


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org