Simplify id space management #908
Conversation
tobim
force-pushed
the
story/ch16628
branch
3 times, most recently
from
3f28929 to
667af2a
Compare
| available << '\n' << i->i << ' ' << i->last; | ||
| } | ||
| VAST_DEBUG(self, "saved", available_ids(), "available IDs"); | ||
| std::ofstream available{to_string(dir / "next_id")}; |
There was a problem hiding this comment.
Can you add a dir.is_writable() check above this line?
There was a problem hiding this comment.
I'm against that. dir should always be a child of a vast.db folder, and that check should only be done for the parent.
There was a problem hiding this comment.
The vast.db may already exist or be created by a system service, and the user may be using the wrong umask. I don't think we can make this assumption.
|
I think we should remove the consensus module from the repo if it's essentially dead code now. |
This change decouples the importer from the consensus and lets it manage the available ids independently. That means that we won't be able to migrate partitions between nodes any longer. However that is currently not an issue since cluster mode is currently not implemented.
If a source wants to block until all its slices were consumed at the index, it needs to subscribe to a flush notification. Before this change, the importer would only forward this subscription to the index if its own buffers were empty already, but not in the other case.
|
@dominiklohmann do you mind taking another look at the latest commit. (The rest is only rebase.) |
|
Looks fine from a taking a look at the code only. I'll check this out and run some tests locally. |
There was a problem hiding this comment.
This works really well in practice.
I think we should revert #906 as part of this PR. It's useful for monitoring whether this change works.
I've noticed a crash when setting table-slice-size to 0, which should probably get a nicer-looking error message instead.
I'd prefer not to, as the algorithm to build the JSON value is still extremely inefficient. (Will give problems with large archives regardless.)
Will do. |
No description provided.