Fix missing dense indexes and blocking import hangs #2848
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
What bug does this fix and why is this the correct solution? Edit: Thanks for updating the description. |
tobim
force-pushed
the
story/sc-40355/fix-partition-stream-routing
branch
3 times, most recently
from
196cc76 to
a38e8dc
Compare
Some additional checks to make sure skipped indexes don't interfere with subsequent dense index creation. The last check is failing as of this commit, because the stream stage in the active partition does not iterate over slice columns correctly.
Dropping the `talbe_slice_column` wrapping for streaming data to active indexers simplifies the streaming logic and removes the need to filter outputs on the qualified record field. The indexers are now spawned with the index of the slice column instead and retrieve the correct data by themself.
tobim
force-pushed
the
story/sc-40355/fix-partition-stream-routing
branch
from
a38e8dc to
247a942
Compare
…partition-stream-routing
…partition-stream-routing
This fixes a bug that was only surfaced by #2807, so it's not really worth a separate changelog entry.
dominiklohmann
approved these changes
Jan 17, 2023
There was a problem hiding this comment.
This is fantastic. As we discussed earlier, I pushed a fix for the broken unit test and the merge conflicts with master directly to the branch. There's only minor feedback on the PR itself.
I ran the integration tests that failed frequently 100 times each with master and with this PR in a release build:
sh build/vast/integration-Release.sh -t "(Arrow Full Data Model|Heterogeneous JSONL Import)"For master, I failed 4/100 times.
With this change, it did not fail a single time.
tobim
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Table columns for which no dense indexes are created cause that the last columns in the same table would also not be indexed.
For example for a schema with fields:
<foo, bar #skip, baz, buz>onlyfooandbazwould have a dense index.If that resulted in no dense index creation at all the
import --blockingmechanism also failed, causing the import process to hang forever.The solution is two fold:
For the index creation, we now avoid the stage filter and
table_slice_columnaltogether and let the indexer retrieve the correct column from the slice. This should make the code easier to understand. Together with that we don't bypass the column counter increment for skipped fields any more, so indexers are spawned with the correct column index.For the blocking import we now wait for batches to be handled by the store instead of any of the connected indexers.