dataroaring commented on code in PR #12160:
URL: https://github.com/apache/doris/pull/12160#discussion_r965432734
##########
be/src/common/config.h:
##########
@@ -852,9 +852,11 @@ CONF_Int32(doris_remote_scanner_thread_pool_queue_size,
"10240");
// This config should be removed when the new scan node is ready.
CONF_Bool(enable_new_scan_node, "false");
+CONF_Int32(tablet_schema_cache_recycle_interval, "86400") // s, one day
+
#ifdef BE_TEST
-// test s3
-CONF_String(test_s3_resource, "resource");
+ // test s3
+ CONF_String(test_s3_resource, "resource");
Review Comment:
Is tab not needed?
##########
be/src/olap/tablet_schema.cpp:
##########
@@ -465,6 +466,11 @@ void TabletSchema::append_column(TabletColumn column, bool
is_dropped_column) {
if (column.is_nullable()) {
_num_null_columns++;
}
+ if (UNLIKELY(column.name() == DELETE_SIGN)) {
+ _delete_sign_idx = _num_columns;
+ } else if (UNLIKELY(column.name() == SEQUENCE_COL)) {
+ _sequence_col_idx = _num_columns;
+ }
Review Comment:
I am not sure delte sign or seq col can be enabled by alter statement. If
a table is created without 2 columns and then they are enabled by a alter
statement. delete sign index and schema does not match for read?
##########
be/src/olap/reader.cpp:
##########
@@ -475,7 +475,7 @@ ColumnPredicate* TabletReader::_parse_to_predicate(
}
ColumnPredicate* TabletReader::_parse_to_predicate(const FunctionFilter&
function_filter) {
- int32_t index = _tablet->field_index(function_filter._col_name);
+ int32_t index = _tablet_schema->field_index(function_filter._col_name);
Review Comment:
tablet->fieled_index is not needed any more? I can find it in tablet.h but I
can not find it in tablet.cpp.
##########
be/src/olap/tablet_schema_cache.h:
##########
@@ -52,6 +55,25 @@ class TabletSchemaCache {
return iter->second;
}
+private:
+ /**
+ * @brief recycle when TabletSchemaSPtr use_count equals 1.
+ */
+ void _recycle() {
+ for (;;) {
+ std::this_thread::sleep_for(
+
std::chrono::seconds(config::tablet_schema_cache_recycle_interval));
Review Comment:
Could we remove this config? There are too many configs in doris? Do we
really need it as a config?
##########
be/src/olap/tablet.cpp:
##########
@@ -99,6 +99,15 @@ Tablet::Tablet(TabletMetaSharedPtr tablet_meta, DataDir*
data_dir,
// construct _timestamped_versioned_tracker from rs and stale rs meta
_timestamped_version_tracker.construct_versioned_tracker(_tablet_meta->all_rs_metas(),
_tablet_meta->all_stale_rs_metas());
+ // if !_tablet_meta->all_rs_metas()[0]->tablet_schema(),
+ // that mean the tablet_meta is still no upgrade to 1.2.
Review Comment:
What does 1.2 mean here? A meta version or rowset's version[1,2). Please
clarify it more.
##########
be/src/olap/tablet_schema_cache.h:
##########
@@ -52,6 +55,25 @@ class TabletSchemaCache {
return iter->second;
}
+private:
+ /**
+ * @brief recycle when TabletSchemaSPtr use_count equals 1.
+ */
+ void _recycle() {
+ for (;;) {
+ std::this_thread::sleep_for(
+
std::chrono::seconds(config::tablet_schema_cache_recycle_interval));
+ std::lock_guard guard(_mtx);
+ for (auto iter = _cache.begin(), last = _cache.end(); iter !=
last;) {
+ if (iter->second.use_count() == 1) {
+ iter = _cache.erase(iter);
+ } else {
+ ++iter;
+ }
+ }
Review Comment:
Recycler does not consider memory consumption and timing. I worry that would
lead to performance drop. May be we should have a performance test with a
smaller enough recycle_interval.
--
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: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
