arrow
arrow copied to clipboard
ARROW-17278: [C++][Benchmarking] Add AsofJoin Ordering Assertion and Benchmark Fixes
- Add a invalid status if batches are ingested in an incorrect order for any source in AsOfJoin.
- Fix benchmarks to use isolated memory pools.
- Create and discard memory pools over different benchmark iterations (if possible) to prevent OOM from creating too many memory pools.
https://issues.apache.org/jira/browse/ARROW-17278
:warning: Ticket has not been started in JIRA, please click 'Start Progress'.
@westonpace Hi Weston,
I wrote some new tests to validate that our in-order assertions are working (it returns an invalid status which is caught by ErrorIfNotOk) . They seem to detect the out-of-ordered test locally on my computer and it passes, but it seems to fail on the CI checks -- any idea why?
I think I found the issue, doesn't seem like it passes always?
EDIT: seems like some sort of race condition. We only run the check when are sure the queue
has batches in it, however, it seems like in the cases in which the tests fails, it is always empty
. The execution context is setup with nullptr
as well. Any ideas here @icexelloss ?
Can you print thread id and see if input received is called by different thread?( for the same input table). Also I think we can check when queue is not empty too, we just need to remember the current timestamp in the input state.
On Tue, Aug 2, 2022 at 5:36 PM Ivan Chau @.***> wrote:
I think I found the issue, doesn't seem like it passes always?
EDIT: seems like some sort of race condition. We only run the check when are sure the queue has batches in it, however, it seems like in the cases in which the tests fails, it is always empty. The execution context is setup with nullptr as well. Any ideas here @icexelloss https://github.com/icexelloss ?
— Reply to this email directly, view it on GitHub https://github.com/apache/arrow/pull/13771#issuecomment-1203227263, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGBXLDCYVT3PCXTQLUBSOLVXGIEZANCNFSM55I6LW6A . You are receiving this because you were mentioned.Message ID: @.***>
Resolved, was checking the last batch in the queue to find latest time, however these batches get consumed and the queue can become empty, instead introduce another instance variable to track latest time seen in the InputState
.
@westonpace - @iChauster has finished his internship last week so I will take this PR over. Let me know if you have more comments.
@westonpace Are there more changes needed for this PR?
@westonpace Anything else we need to do here for merging?
Closing because it has been untouched for a while, in case it's still relevant feel free to reopen and move it forward 👍