gavinchou commented on code in PR #53540:
URL: https://github.com/apache/doris/pull/53540#discussion_r2254149464
##########
be/src/olap/tablet_meta.h:
##########
@@ -247,7 +247,11 @@ class TabletMeta : public MetadataAdder<TabletMeta> {
void remove_rowset_delete_bitmap(const RowsetId& rowset_id, const Version&
version);
bool enable_unique_key_merge_on_write() const { return
_enable_unique_key_merge_on_write; }
-
+#ifdef BE_TEST
Review Comment:
no need to `ifdef` for the test code.
we can acces any private members of a class in the testing code directly
with operator `.` or `->`
##########
be/src/olap/version_graph.h:
##########
@@ -168,6 +184,21 @@ class TimestampedVersionTracker {
Status capture_consistent_versions(const Version& spec_version,
std::vector<Version>* version_path)
const;
+ // Given a start, this method can find a version path which satisfy the
following conditions:
+ // 1. all edges satisfy the conditions specified by `validator` in the
graph.
+ // 2. the destination version is as far as possible.
+ // 3. the path is the shortest path.
+ // The version paths are added to version_path as return info.
+ // If this version not in main version, version_path can be included
expired rowset.
+ // NOTE: this method may return edges which is in stale path
+ Status capture_consistent_versions_with_validator(
+ int64_t start, std::vector<Version>& version_path,
+ const std::function<bool(int64_t, int64_t)>& validator) const;
+
+ Status capture_newest_consistent_versions_with_validator(
+ int64_t start, std::vector<Version>& version_path,
+ const std::function<bool(int64_t, int64_t)>& validator) const;
+
Review Comment:
what is the behavior of a `validator`, what does it expect for the input
param?
##########
be/src/cloud/cloud_tablet.cpp:
##########
@@ -127,6 +141,73 @@ Status CloudTablet::capture_rs_readers(const Version&
spec_version,
return capture_rs_readers_unlocked(version_path, rs_splits);
}
+Status CloudTablet::capture_rs_readers_with_freshness_tolerance(
+ const Version& spec_version, std::vector<RowSetSplits>* rs_splits,
+ bool skip_missing_version, int64_t query_freshness_tolerance_ms) {
+ g_capture_with_freshness_tolerance_count << 1;
+ using namespace std::chrono;
+ auto freshness_limit_tp = system_clock::now() -
milliseconds(query_freshness_tolerance_ms);
+ auto startup_timepoint = _engine.startup_timepoint();
+ // find a version path where every edge(rowset) has been warmuped
+ auto rowset_is_warmed_up = [&](int64_t start_version, int64_t end_version)
-> bool {
+ if (start_version > end_version) {
+ return false;
Review Comment:
is this status type intent?
this lambda returns `bool`
##########
cloud/src/meta-service/meta_service_job.cpp:
##########
@@ -1051,9 +1051,22 @@ void process_compaction_job(MetaServiceCode& code,
std::string& msg, std::string
INSTANCE_LOG(INFO) << "remove tmp rowset meta, tablet_id=" << tablet_id
<< " tmp_rowset_key=" << hex(tmp_rowset_key);
+ using namespace std::chrono;
+ auto rowset_visible_time =
+
duration_cast<milliseconds>(system_clock::now().time_since_epoch()).count();
+ rs_meta.set_visible_time_ms(rowset_visible_time);
Review Comment:
remove the comment of line 1039
##########
be/src/olap/rowset/rowset_meta.h:
##########
@@ -348,6 +349,22 @@ class RowsetMeta : public MetadataAdder<RowsetMeta> {
int64_t newest_write_timestamp() const { return
_rowset_meta_pb.newest_write_timestamp(); }
+ // for cloud only
+ bool has_visible_time_ms() const { return
_rowset_meta_pb.has_visible_time_ms(); }
+ int64_t visible_time_ms() const { return
_rowset_meta_pb.visible_time_ms(); }
Review Comment:
no need to ifdef for the test code.
we can acces any private members of a class in the testing code directly
with operator . or ->
##########
gensrc/proto/olap_file.proto:
##########
@@ -148,6 +148,8 @@ message RowsetMetaPB {
optional SchemaDictKeyList schema_dict_key_list = 1008; // align to cloud
rowset
optional SplitSchemaPB __split_schema = 1009; // A special field, DO NOT
change it.
+
+ optional int64 visible_time_ms = 1010;
Review Comment:
这个是什么时间?如果是时间戳建议叫 visible_ts_ms
##########
be/src/cloud/cloud_tablet.h:
##########
@@ -63,6 +63,11 @@ class CloudTablet final : public BaseTablet {
Status capture_rs_readers(const Version& spec_version,
std::vector<RowSetSplits>* rs_splits,
bool skip_missing_version) override;
+ Status capture_rs_readers_with_freshness_tolerance(const Version&
spec_version,
Review Comment:
add detailed comment: behavior an params and return value
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
