Add multi-path in RowsetGraph #1761

chaoyli · 2019-09-08T13:44:41Z

Do not remove rowset from RowsetGraph after compaction
Collect unused rowset at interval
Remove inc_version and related code

chaoyli · 2019-09-08T13:45:16Z

Related issue is #1759

morningman · 2019-09-08T14:35:55Z

be/src/olap/tablet.cpp

    vector<pair<Version, VersionHash>> existing_versions;
    for (auto& rs : _tablet_meta->all_rs_metas()) {
+        LOG(INFO) << "rs version:" << rs->version().first


Should use DEBUG level bug

morningman · 2019-09-08T14:38:19Z

be/src/olap/tablet.h

@@ -233,6 +228,7 @@ class Tablet : public std::enable_shared_from_this<Tablet> {
    // eco mode also means save money in palo
    inline bool in_eco_mode() { return false; }

+    OLAPStatus capture_unused_rowsets();


Add comment to explain this function

morningman · 2019-09-08T14:43:58Z

be/src/olap/olap_server.cpp

@@ -40,11 +40,17 @@ namespace doris {
 volatile uint32_t g_schema_change_active_threads = 0;

 OLAPStatus StorageEngine::_start_bg_worker() {


Add comment to explain what are these bg thread doing

imay · 2019-09-09T06:42:30Z

be/src/olap/tablet.cpp

@@ -970,4 +878,42 @@ void Tablet::pick_candicate_rowsets_to_base_compaction(std::vector<RowsetSharedP
    }
 }

+OLAPStatus Tablet::capture_unused_rowsets() {


You will iterate all tablet every time with lock, and I think it can be improved.

yiguolei · 2019-09-09T09:14:37Z

be/src/olap/olap_snapshot_converter.cpp

@@ -68,11 +68,6 @@ OLAPStatus OlapSnapshotConverter::to_olap_header(const TabletMetaPB& tablet_meta
        PDelta* pdelta = olap_header->add_delta();
        convert_to_pdelta(rs_meta, pdelta);
    }
-    // not add pending delta, it is usedless in clone or backup restore
-    for (auto& inc_rs_meta : tablet_meta_pb.inc_rs_metas()) {
-        PDelta* pdelta = olap_header->add_incremental_delta();


This code is useful in clone from new BE to old BE.

yiguolei · 2019-09-09T09:15:26Z

be/src/olap/olap_snapshot_converter.cpp

@@ -129,20 +124,6 @@ OLAPStatus OlapSnapshotConverter::to_tablet_meta_pb(const OLAPHeaderMessage& ola
        _rs_version_map[rowset_version] = rowset_meta;
    }

-    for (auto& inc_delta : olap_header.incremental_delta()) {
-        // check if inc delta already exist in delta


It is useful when upgrade from old BE to new BE.

yiguolei · 2019-09-09T10:27:00Z

be/src/olap/snapshot_manager.cpp

            vector<RowsetMetaSharedPtr> empty_rowsets;
            new_tablet_meta->revise_rs_metas(empty_rowsets);
        } else {
            // If this is a full clone, then should clear inc rowset metas because 
            // related files is not created
            vector<RowsetMetaSharedPtr> empty_rowsets;
-            new_tablet_meta->revise_inc_rs_metas(empty_rowsets);
            new_tablet_meta->revise_rs_metas(rs_metas);


revise is not useful any more. should remove it.

yiguolei · 2019-09-09T10:46:19Z

be/src/olap/tablet_meta.cpp

-}
-
-RowsetMetaSharedPtr TabletMeta::acquire_inc_rs_meta_by_version(const Version& version) const {
-    RowsetMetaSharedPtr rs_meta = nullptr;


It is used in incremental clone, if it is removed the clone logic may run to error.

yiguolei · 2019-09-10T02:58:37Z

be/src/olap/schema_change.cpp

@@ -1250,7 +1250,7 @@ OLAPStatus SchemaChangeHandler::_do_process_alter_tablet_v2(const TAlterTabletRe
                rowsets_to_delete.push_back(rowset);
            }
        }
-        new_tablet->modify_rowsets(std::vector<RowsetSharedPtr>(), rowsets_to_delete);
+        new_tablet->delete_rowsets(rowsets_to_delete);


I think there should be only one method calling delete rowsets. it is capture unused rowset.

yiguolei · 2019-09-10T03:00:00Z

be/src/olap/snapshot_manager.cpp

@@ -438,14 +422,12 @@ OLAPStatus SnapshotManager::_create_snapshot_files(
        new_tablet_meta->delete_alter_task();

        if (request.__isset.missing_version) {
-            new_tablet_meta->revise_inc_rs_metas(rs_metas);


Clone will always failed

yiguolei · 2019-09-10T05:59:52Z

be/src/olap/tablet.cpp

+    return OLAP_SUCCESS;
+}
+
+OLAPStatus Tablet::delete_rowsets(const vector<RowsetSharedPtr>& to_delete) {
    vector<RowsetMetaSharedPtr> rs_metas_to_delete;


delete rowsets should add the rowset to unused rowset list

morningman · 2019-09-10T07:06:58Z

be/src/olap/tablet.cpp

    vector<pair<Version, VersionHash>> existing_versions;
    for (auto& rs : _tablet_meta->all_rs_metas()) {
+        LOG(INFO) << "rs version:" << rs->version().first
+                  << "-" << rs->version().second;
        existing_versions.emplace_back(rs->version() , rs->version_hash());


Suggested change

existing_versions.emplace_back(rs->version() , rs->version_hash());

existing_versions.emplace_back(rs->version(), rs->version_hash());

morningman · 2019-09-10T07:25:04Z

in src//olap/tablet.cpp, the function capture_consistent_versions() which will print WANRNING log when version missing. But these is in critical read path of Doris, so I think WARNING log can not be printed here.

yiguolei

some comments

…ns for dynamic table (apache#1761)

morningman changed the title ~~And multi-path in RowsetGraph~~ Add multi-path in RowsetGraph Sep 8, 2019

morningman requested changes Sep 8, 2019

View reviewed changes

imay reviewed Sep 9, 2019

View reviewed changes

yiguolei reviewed Sep 9, 2019

View reviewed changes

yiguolei reviewed Sep 10, 2019

View reviewed changes

morningman reviewed Sep 10, 2019

View reviewed changes

yiguolei reviewed Sep 10, 2019

View reviewed changes

morningman added this to TODO in Decoupling storage from computation via automation Sep 11, 2019

Limit the max version to cumulative compaction

d60331e

chaoyli closed this Sep 16, 2019

SWJTU-ZhangLei pushed a commit to SWJTU-ZhangLei/incubator-doris that referenced this pull request Jul 25, 2023

[regression-test](dynamic table) add check for counting rows of colum…

a22c4ae

…ns for dynamic table (apache#1761)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add multi-path in RowsetGraph #1761

Add multi-path in RowsetGraph #1761

chaoyli commented Sep 8, 2019

chaoyli commented Sep 8, 2019 •

edited

morningman Sep 8, 2019

morningman Sep 8, 2019

morningman Sep 8, 2019

imay Sep 9, 2019

yiguolei Sep 9, 2019

yiguolei Sep 9, 2019

yiguolei Sep 9, 2019

yiguolei Sep 9, 2019

yiguolei Sep 10, 2019

yiguolei Sep 10, 2019

yiguolei Sep 10, 2019

morningman Sep 10, 2019

morningman commented Sep 10, 2019

yiguolei left a comment

		@@ -40,11 +40,17 @@ namespace doris {
		volatile uint32_t g_schema_change_active_threads = 0;

		OLAPStatus StorageEngine::_start_bg_worker() {

	existing_versions.emplace_back(rs->version() , rs->version_hash());
	existing_versions.emplace_back(rs->version(), rs->version_hash());

Add multi-path in RowsetGraph #1761

Add multi-path in RowsetGraph #1761

Conversation

chaoyli commented Sep 8, 2019

chaoyli commented Sep 8, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

morningman commented Sep 10, 2019

yiguolei left a comment

Choose a reason for hiding this comment

chaoyli commented Sep 8, 2019 •

edited