gavinchou commented on code in PR #31055: URL: https://github.com/apache/doris/pull/31055#discussion_r1494370069
########## fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeJobV2.java: ########## @@ -105,42 +105,52 @@ public class SchemaChangeJobV2 extends AlterJobV2 { private Map<Long, String> indexIdToName = Maps.newHashMap(); // shadow index id -> index schema @SerializedName(value = "indexSchemaMap") - private Map<Long, List<Column>> indexSchemaMap = Maps.newHashMap(); + protected Map<Long, List<Column>> indexSchemaMap = Maps.newHashMap(); // shadow index id -> (shadow index schema version : schema hash) @SerializedName(value = "indexSchemaVersionAndHashMap") - private Map<Long, SchemaVersionAndHash> indexSchemaVersionAndHashMap = Maps.newHashMap(); + protected Map<Long, SchemaVersionAndHash> indexSchemaVersionAndHashMap = Maps.newHashMap(); // shadow index id -> shadow index short key count @SerializedName(value = "indexShortKeyMap") - private Map<Long, Short> indexShortKeyMap = Maps.newHashMap(); + protected Map<Long, Short> indexShortKeyMap = Maps.newHashMap(); // bloom filter info @SerializedName(value = "hasBfChange") private boolean hasBfChange; @SerializedName(value = "bfColumns") - private Set<String> bfColumns = null; + protected Set<String> bfColumns = null; @SerializedName(value = "bfFpp") - private double bfFpp = 0; + protected double bfFpp = 0; // alter index info @SerializedName(value = "indexChange") private boolean indexChange = false; @SerializedName(value = "indexes") - private List<Index> indexes = null; + protected List<Index> indexes = null; @SerializedName(value = "storageFormat") private TStorageFormat storageFormat = TStorageFormat.DEFAULT; + @SerializedName(value = "isCloudSchemaChange") + private boolean isCloudSchemaChange = false; Review Comment: mark, consider removing it ########## fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeJobV2.java: ########## @@ -962,6 +991,14 @@ private void changeTableState(long dbId, long tableId, OlapTableState olapTableS } } + protected void commitShadowIndex() throws AlterCancelException {} Review Comment: add comment for these methods ########## fe/fe-core/src/main/java/org/apache/doris/catalog/Replica.java: ########## @@ -511,6 +528,7 @@ public synchronized void updateLastFailedVersion(long lastFailedVersion) { * is already updated by load process, so we need to consider its version. */ public boolean checkVersionCatchUp(long expectedVersion, boolean ignoreAlter) { + LOG.info("????"); Review Comment: remove if not need ########## be/src/olap/schema_change.cpp: ########## @@ -507,42 +501,30 @@ VSchemaChangeWithSorting::VSchemaChangeWithSorting(const BlockChanger& changer, fmt::format("VSchemaChangeWithSorting:changer={}", std::to_string(int64(&changer)))); } -Status VSchemaChangeWithSorting::_inner_process(RowsetReaderSharedPtr rowset_reader, +Status VBaseSchemaChangeWithSorting::_inner_process(RowsetReaderSharedPtr rowset_reader, RowsetWriter* rowset_writer, - TabletSharedPtr new_tablet, + BaseTabletSPtr new_tablet, TabletSchemaSPtr base_tablet_schema, TabletSchemaSPtr new_tablet_schema) { + LOG_INFO("lightman VBaseSchemaChangeWithSorting::_inner_process"); Review Comment: remove debug log ########## be/src/olap/schema_change.h: ########## @@ -196,26 +197,31 @@ class VSchemaChangeDirectly : public SchemaChange { const BlockChanger& _changer; }; -// @breif schema change with sorting -class VSchemaChangeWithSorting : public SchemaChange { +class VBaseSchemaChangeWithSorting : public SchemaChange { Review Comment: What does the "Base" mean? ########## be/src/cloud/cloud_schema_change_job.h: ########## @@ -0,0 +1,35 @@ +#pragma once + +#include <memory> +#include "cloud/cloud_storage_engine.h" +#include "olap/rowset/rowset.h" +#include "olap/schema_change.h" +#include "cloud/cloud_tablet.h" +#include "olap/tablet_fwd.h" + +namespace doris { + +class CloudSchemaChangeJob { +public: + CloudSchemaChangeJob(CloudStorageEngine& cloud_storage_engine, std::string job_id, int64_t expiration); + ~CloudSchemaChangeJob(); + + // This method is idempotent for a same request. + Status process_alter_tablet(const TAlterTabletReqV2& request); + +private: + Status _convert_historical_rowsets(const SchemaChangeParams& sc_params); + +private: + CloudStorageEngine& _cloud_storage_engine; + std::shared_ptr<CloudTablet> _base_tablet; + std::shared_ptr<CloudTablet> _new_tablet; + TabletSchemaSPtr _base_tablet_schema; + TabletSchemaSPtr _new_tablet_schema; + std::string _job_id; + std::vector<RowsetSharedPtr> _output_rowsets; + int64_t _output_cumulative_point = 0; + int64_t _expiration; Review Comment: add comment to describe what it is for -- 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