Skip to content

Conversation

@ZhangYu0123
Copy link
Contributor

@ZhangYu0123 ZhangYu0123 commented Jul 7, 2020

Related issue #4017, main changes as follows:

  1. Add expired_snapshot_rs_version_map,_expired_snapshot_rs_metas,
  2. Add VersionedRowsetTracker record compacted path version
  3. Record path version when rowsets compact
  4. In gc process, add expired snapshot rowsets to unused set to remove.

_rs_graph.construct_rowset_graph(_tablet_meta->all_rs_metas());
// change _rs_graph to _versioned_rs_tracker
// _rs_graph.construct_rowset_graph(_tablet_meta->all_rs_metas());
_versioned_rs_tracker.construct_versioned_tracker(_tablet_meta->all_rs_metas(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

66 - 67 has nonstandard format

@ZhangYu0123 ZhangYu0123 force-pushed the merged_rowset_delay_unused branch from 0f8d14d to 5eb43d3 Compare July 7, 2020 12:17
@morningman morningman added area/storage Issues or PRs related to storage engine kind/improvement labels Jul 7, 2020
RETURN_NOT_OK(gc_unused_rowsets());
TRACE("unused rowsets have been moved to GC queue");
// RETURN_NOT_OK(gc_unused_rowsets());
// TRACE("unused rowsets have been moved to GC queue");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove it

}

void RowsetGraph::construct_rowset_graph(const std::vector<RowsetMetaSharedPtr>& rs_metas,
int64_t& max_version) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
int64_t& max_version) {
int64_t* max_version) {

Better to use point to indicate it is a output parameter.


/// RowsetGraph class which is implemented to build and maintain total versions of rowsets.
/// This class use adjacency-matrix represent rowsets version and links. A vertex is a version
/// and a link is the _version object of a rowset (from start version to end version).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// and a link is the _version object of a rowset (from start version to end version).
/// and a link is the _version object of a rowset (from start version to end version + 1).

};

} // namespace doris
/// VersionTracker class which is implemented to maintain compacted version path of rowsets.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// VersionTracker class which is implemented to maintain compacted version path of rowsets.
/// VersionTracker class which is implemented to maintain multi-version path of rowsets.

/// VersionedRowsetTracker class is responsible to track all rowsets version links of a tablet.
/// This class not only records the graph of all versions, but also records the paths which will be removed
/// after the path is expired.
class VersionedRowsetTracker {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
class VersionedRowsetTracker {
class TimestampedVersionTracker {

// 2 remove this path from other_path
auto path_iter = path_version_ptr->begin();
for (; path_iter != path_version_ptr->end(); path_iter++) {
Version version = (*path_iter)->version();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Version version = (*path_iter)->version();
const Version& version = (*path_iter)->version();

_construct_versioned_tracker(rs_metas, expired_snapshot_rs_metas);
}

void VersionedRowsetTracker::reconstruct_versioned_tracker(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks same as construct_versioned_tracker()


while (iter != _expired_snapshot_rs_path_map.end()) {
std::vector<VersionTrackerSharedPtr>::iterator version_path_iter = iter->second->begin();
int64_t max_create_time = -1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can save the max_create_time of a version path in _expired_snapshot_rs_path_map

}
}

void VersionedRowsetTracker::_print_current_state() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to return a string, and let the caller decide how to print it.

@ZhangYu0123 ZhangYu0123 force-pushed the merged_rowset_delay_unused branch from 3ed0620 to 64713d0 Compare July 15, 2020 16:56
Copy link
Contributor

@morningman morningman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@morningman morningman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@morningman morningman added the approved Indicates a PR has been approved by one committer. label Jul 19, 2020
@morningman morningman merged commit 03cf9b2 into apache:master Jul 19, 2020
@EmmyMiao87 EmmyMiao87 mentioned this pull request Aug 17, 2020
csun5285 added a commit to csun5285/doris that referenced this pull request Sep 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. area/storage Issues or PRs related to storage engine kind/improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants