fix(MemTable): make it cancel-safe and fix parallelism #5197
Conversation
| data.push(result.map_err(|_| { | ||
| DataFusionError::Internal( | ||
| "MemTable::load could not join task".to_string(), | ||
| ) |
There was a problem hiding this comment.
I think it would help if we could preserve the original error here rather than turning it into an internal error
Perhaps using DataFusionError::Context
| data.push(result.map_err(|_| { | |
| DataFusionError::Internal( | |
| "MemTable::load could not join task".to_string(), | |
| ) | |
| data.push(result.map_err(|e| { | |
| DataFusionError::Context( | |
| "MemTable::load could not join task".to_string(), Box::new(e) | |
| ) |
There was a problem hiding this comment.
Context requires DataFusionError, so it's anyway should be wrapped to something 🤔
Could be data.push(result.map_err(|e| DataFusionError::External(Box::new(e)))??), WDYT?
There was a problem hiding this comment.
That would definitely be better than 👍
|
Benchmark runs are scheduled for baseline = 816a0f8 and contender = f63b972. f63b972 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
3 participants
Which issue does this PR close?
The issue is close to #5178, but for
MemTable.Rationale for this change
awaitwas sequentialWhat changes are included in this PR?
Are these changes tested?
Existed tests are passing
Are there any user-facing changes?