Skip to content

Commit

Permalink
[BugFix][Cherry-pick][Branch-3.0] Recalculate max_continuous_version …
Browse files Browse the repository at this point in the history
…after schema change finished (#28473) (#28522)

We will create a new tablet for each original tablet and we will write
original tablet and new tablet during schema change. We will construct a
`version_graph` to keep versions info for each table and the following
code is how to update `version_graph`:
```
void VersionGraph::add_version_to_graph(const Version& version) {
    _add_version_to_graph(version);
    if (version.first == _max_continuous_version + 1) {
        _max_continuous_version = _get_max_continuous_version_from(_max_continuous_version + 1);
    } else if (version.first == 0) {
        // We need to reconstruct max_continuous_version from zero if input version is starting from zero
        // e.g.
        // 1. Tablet A is doing schema change
        // 2. We create a new tablet B releated A, and we will create a initial rowset and _max_continuous_version
        //    will be updated to 1
        // 3. Tablet A has a rowset R with version (0, m)
        // 4. Schema change will try convert R
        // 5. The start version of R (0) is not equal to `_max_continuous_version + 1`, and the _max_continuous_version
        //    will not update
        _max_continuous_version = _get_max_continuous_version_from(0);
    }
}
```
There exists a scenario where _max_continuous_version cannot be updated
correctly
e.g.
1. create a new tablet `t2` for origin tablet `t1` during schema change
and `t2` has version 91,92,93,95,96,97,98. version 94 write failed.
2. when running schema change job, alter version is 98 and `t1` has
version [0-90], [91-96], 97, 98
3. `max_continuous_version` of `t2` is 0 before convert rowset. After
convert version [0-90], `max_continuous_version` is update to 93 because
`t2` has version 91-93 before.
4. `max_continuous_version` don't update because the left rowsets
version is not satisfy update condition(version.fisrt == 0 or
version.first == _max_continuous_verison + 1). So the
`max_continuous_version` will keep 93 after convert rowset finished.
5. we will check the max_continuous_version of `t2` at last and it
should not less than alter version. But max_continuous_version(93) is
less than alter version(98), so the alter job failed at last.

The main reason is we don't update `_max_continuous_version` correctly
during alter job, we should recalculate the `max_continuous_version`
after rowset conversion.

---------

Signed-off-by: zhangqiang <[email protected]>
  • Loading branch information
sevev authored Aug 3, 2023
1 parent dd938bf commit e50b34b
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 0 deletions.
1 change: 1 addition & 0 deletions be/src/storage/schema_change.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1089,6 +1089,7 @@ Status SchemaChangeHandler::_convert_historical_rowsets(SchemaChangeParams& sc_p
if (status.ok()) {
status = sc_params.new_tablet->check_version_integrity(sc_params.version);
}
sc_params.new_tablet->update_max_continuous_version();

LOG(INFO) << "finish converting rowsets for new_tablet from base_tablet. "
<< "base_tablet=" << sc_params.base_tablet->full_name()
Expand Down
2 changes: 2 additions & 0 deletions be/src/storage/tablet.h
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,8 @@ class Tablet : public BaseTablet {

void get_basic_info(TabletBasicInfo& info);

void update_max_continuous_version() { _timestamped_version_tracker.update_max_continuous_version(); }

protected:
void on_shutdown() override;

Expand Down
8 changes: 8 additions & 0 deletions be/src/storage/version_graph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@ int64_t TimestampedVersionTracker::get_max_continuous_version() const {
return _version_graph.max_continuous_version();
}

void TimestampedVersionTracker::update_max_continuous_version() {
_version_graph.update_max_continuous_version();
}

int64_t TimestampedVersionTracker::get_min_readable_version() const {
return _version_graph.min_readable_version();
}
Expand Down Expand Up @@ -236,6 +240,10 @@ std::vector<TimestampedVersionSharedPtr>& TimestampedVersionPathContainer::times
return _timestamped_versions_container;
}

void VersionGraph::update_max_continuous_version() {
_max_continuous_version = _get_max_continuous_version_from(0);
}

void VersionGraph::construct_version_graph(const std::vector<RowsetMetaSharedPtr>& rs_metas, int64_t* max_version) {
_version_graph.clear();
_max_continuous_version = -1;
Expand Down
4 changes: 4 additions & 0 deletions be/src/storage/version_graph.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ class VersionGraph {
// Get max continuous version from 0
int64_t max_continuous_version() const { return _max_continuous_version; }

void update_max_continuous_version();

int64_t min_readable_version() const { return _min_readable_version; }

private:
Expand Down Expand Up @@ -211,6 +213,8 @@ class TimestampedVersionTracker {
// Get max continuous version from 0
int64_t get_max_continuous_version() const;

void update_max_continuous_version();

int64_t get_min_readable_version() const;

private:
Expand Down

0 comments on commit e50b34b

Please sign in to comment.