You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/04/14 09:48:35 UTC
[GitHub] [incubator-doris] EmmyMiao87 commented on a diff in pull request #8858: [feature-wip](statistics) step1: create the statistics job
EmmyMiao87 commented on code in PR #8858:
URL: https://github.com/apache/incubator-doris/pull/8858#discussion_r850200516
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyzeStmt.java:
##########
@@ -17,19 +17,181 @@
package org.apache.doris.analysis;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Column;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.cluster.ClusterNamespace;
+import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.Config;
+import org.apache.doris.common.ErrorCode;
+import org.apache.doris.common.ErrorReport;
+import org.apache.doris.common.UserException;
+import org.apache.doris.mysql.privilege.PaloAuth;
+import org.apache.doris.mysql.privilege.PrivPredicate;
+import org.apache.doris.qe.ConnectContext;
+
+import com.clearspring.analytics.util.Lists;
+import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
+import com.google.common.collect.Maps;
+
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
/**
* Collect statistics about a database
- *
+ * <p>
* syntax:
* ANALYZE [[ db_name.tb_name ] [( column_name [, ...] )], ...] [ PROPERTIES(...) ]
- *
- * db_name.tb_name: collect table and column statistics from tb_name
- *
- * column_name: collect column statistics from column_name
- *
- * properties: properties of statistics jobs
- *
+ * <p>
Review Comment:
Please keep the indentation
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyzeStmt.java:
##########
@@ -17,19 +17,181 @@
package org.apache.doris.analysis;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Column;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.cluster.ClusterNamespace;
+import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.Config;
+import org.apache.doris.common.ErrorCode;
+import org.apache.doris.common.ErrorReport;
+import org.apache.doris.common.UserException;
+import org.apache.doris.mysql.privilege.PaloAuth;
+import org.apache.doris.mysql.privilege.PrivPredicate;
+import org.apache.doris.qe.ConnectContext;
+
+import com.clearspring.analytics.util.Lists;
Review Comment:
error import . Please use 'import com.google.common.collect.Lists;'
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyzeStmt.java:
##########
@@ -17,19 +17,181 @@
package org.apache.doris.analysis;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Column;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.cluster.ClusterNamespace;
+import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.Config;
+import org.apache.doris.common.ErrorCode;
+import org.apache.doris.common.ErrorReport;
+import org.apache.doris.common.UserException;
+import org.apache.doris.mysql.privilege.PaloAuth;
+import org.apache.doris.mysql.privilege.PrivPredicate;
+import org.apache.doris.qe.ConnectContext;
+
+import com.clearspring.analytics.util.Lists;
+import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
+import com.google.common.collect.Maps;
+
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
/**
* Collect statistics about a database
- *
+ * <p>
* syntax:
* ANALYZE [[ db_name.tb_name ] [( column_name [, ...] )], ...] [ PROPERTIES(...) ]
- *
- * db_name.tb_name: collect table and column statistics from tb_name
- *
- * column_name: collect column statistics from column_name
- *
- * properties: properties of statistics jobs
- *
+ * <p>
+ * db_name.tb_name: collect table and column statistics from tb_name
+ * <p>
+ * column_name: collect column statistics from column_name
+ * <p>
+ * properties: properties of statistics jobs
*/
public class AnalyzeStmt extends DdlStmt {
+ private final TableName dbTableName;
+ private final List<String> columnNames;
+ private Map<String, String> properties;
+
+ // after analyzed
+ private Database db;
+ private List<Table> tables;
+ private final Map<Long, List<String>> tableIdToColumnName = Maps.newHashMap();
+
+ public AnalyzeStmt(TableName dbTableName, List<String> columns, Map<String, String> properties) {
+ this.dbTableName = dbTableName;
+ this.columnNames = columns;
+ this.properties = properties;
+ }
+
+ public Database getDb() {
+ Preconditions.checkArgument(isAnalyzed(),
+ "The db name must be obtained after the parsing is complete");
+ return this.db;
+ }
+
+ public List<Table> getTables() {
+ Preconditions.checkArgument(isAnalyzed(),
+ "The db name must be obtained after the parsing is complete");
+ return this.tables;
+ }
+
+ public Map<Long, List<String>> getTableIdToColumnName() {
+ Preconditions.checkArgument(isAnalyzed(),
+ "The db name must be obtained after the parsing is complete");
+ return this.tableIdToColumnName;
+ }
+
+ public Map<String, String> getProperties() {
+ return this.properties;
+ }
+ @Override
+ public void analyze(Analyzer analyzer) throws UserException {
+ super.analyze(analyzer);
+
+ // step1: analyze database and table
+ if (this.dbTableName != null) {
+ String dbName;
Review Comment:
```dbTableName.analyze(analyzer) ```
instead of
line 101 to 113
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyzeStmt.java:
##########
@@ -17,19 +17,181 @@
package org.apache.doris.analysis;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Column;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.cluster.ClusterNamespace;
+import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.Config;
+import org.apache.doris.common.ErrorCode;
+import org.apache.doris.common.ErrorReport;
+import org.apache.doris.common.UserException;
+import org.apache.doris.mysql.privilege.PaloAuth;
+import org.apache.doris.mysql.privilege.PrivPredicate;
+import org.apache.doris.qe.ConnectContext;
+
+import com.clearspring.analytics.util.Lists;
+import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
+import com.google.common.collect.Maps;
+
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
/**
* Collect statistics about a database
- *
+ * <p>
* syntax:
* ANALYZE [[ db_name.tb_name ] [( column_name [, ...] )], ...] [ PROPERTIES(...) ]
- *
- * db_name.tb_name: collect table and column statistics from tb_name
- *
- * column_name: collect column statistics from column_name
- *
- * properties: properties of statistics jobs
- *
+ * <p>
+ * db_name.tb_name: collect table and column statistics from tb_name
+ * <p>
+ * column_name: collect column statistics from column_name
+ * <p>
+ * properties: properties of statistics jobs
*/
public class AnalyzeStmt extends DdlStmt {
+ private final TableName dbTableName;
+ private final List<String> columnNames;
+ private Map<String, String> properties;
+
+ // after analyzed
+ private Database db;
+ private List<Table> tables;
+ private final Map<Long, List<String>> tableIdToColumnName = Maps.newHashMap();
+
+ public AnalyzeStmt(TableName dbTableName, List<String> columns, Map<String, String> properties) {
+ this.dbTableName = dbTableName;
+ this.columnNames = columns;
+ this.properties = properties;
+ }
+
+ public Database getDb() {
+ Preconditions.checkArgument(isAnalyzed(),
+ "The db name must be obtained after the parsing is complete");
+ return this.db;
+ }
+
+ public List<Table> getTables() {
+ Preconditions.checkArgument(isAnalyzed(),
+ "The db name must be obtained after the parsing is complete");
+ return this.tables;
+ }
+
+ public Map<Long, List<String>> getTableIdToColumnName() {
+ Preconditions.checkArgument(isAnalyzed(),
+ "The db name must be obtained after the parsing is complete");
+ return this.tableIdToColumnName;
+ }
+
+ public Map<String, String> getProperties() {
+ return this.properties;
+ }
+ @Override
+ public void analyze(Analyzer analyzer) throws UserException {
+ super.analyze(analyzer);
+
+ // step1: analyze database and table
+ if (this.dbTableName != null) {
+ String dbName;
+ if (Strings.isNullOrEmpty(this.dbTableName.getDb())) {
+ dbName = analyzer.getDefaultDb();
+ } else {
+ dbName = ClusterNamespace.getFullName(analyzer.getClusterName(), this.dbTableName.getDb());
+ }
+
+ // check db
+ if (Strings.isNullOrEmpty(dbName)) {
Review Comment:
Please
1. Check the valid of metadata
2. Check the permission
The logic of the two should not be interspersed and executed together
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyzeStmt.java:
##########
@@ -17,19 +17,181 @@
package org.apache.doris.analysis;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Column;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.cluster.ClusterNamespace;
+import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.Config;
+import org.apache.doris.common.ErrorCode;
+import org.apache.doris.common.ErrorReport;
+import org.apache.doris.common.UserException;
+import org.apache.doris.mysql.privilege.PaloAuth;
+import org.apache.doris.mysql.privilege.PrivPredicate;
+import org.apache.doris.qe.ConnectContext;
+
+import com.clearspring.analytics.util.Lists;
+import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
+import com.google.common.collect.Maps;
+
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
/**
* Collect statistics about a database
- *
+ * <p>
* syntax:
* ANALYZE [[ db_name.tb_name ] [( column_name [, ...] )], ...] [ PROPERTIES(...) ]
- *
- * db_name.tb_name: collect table and column statistics from tb_name
- *
- * column_name: collect column statistics from column_name
- *
- * properties: properties of statistics jobs
- *
+ * <p>
+ * db_name.tb_name: collect table and column statistics from tb_name
+ * <p>
+ * column_name: collect column statistics from column_name
+ * <p>
+ * properties: properties of statistics jobs
*/
public class AnalyzeStmt extends DdlStmt {
+ private final TableName dbTableName;
+ private final List<String> columnNames;
+ private Map<String, String> properties;
+
+ // after analyzed
+ private Database db;
+ private List<Table> tables;
+ private final Map<Long, List<String>> tableIdToColumnName = Maps.newHashMap();
+
+ public AnalyzeStmt(TableName dbTableName, List<String> columns, Map<String, String> properties) {
+ this.dbTableName = dbTableName;
+ this.columnNames = columns;
+ this.properties = properties;
+ }
+
+ public Database getDb() {
+ Preconditions.checkArgument(isAnalyzed(),
+ "The db name must be obtained after the parsing is complete");
+ return this.db;
+ }
+
+ public List<Table> getTables() {
+ Preconditions.checkArgument(isAnalyzed(),
+ "The db name must be obtained after the parsing is complete");
+ return this.tables;
+ }
+
+ public Map<Long, List<String>> getTableIdToColumnName() {
+ Preconditions.checkArgument(isAnalyzed(),
+ "The db name must be obtained after the parsing is complete");
+ return this.tableIdToColumnName;
+ }
+
+ public Map<String, String> getProperties() {
+ return this.properties;
+ }
+ @Override
+ public void analyze(Analyzer analyzer) throws UserException {
+ super.analyze(analyzer);
+
+ // step1: analyze database and table
+ if (this.dbTableName != null) {
+ String dbName;
+ if (Strings.isNullOrEmpty(this.dbTableName.getDb())) {
+ dbName = analyzer.getDefaultDb();
+ } else {
+ dbName = ClusterNamespace.getFullName(analyzer.getClusterName(), this.dbTableName.getDb());
+ }
+
+ // check db
+ if (Strings.isNullOrEmpty(dbName)) {
+ ErrorReport.reportAnalysisException(ErrorCode.ERR_NO_DB_ERROR);
+ }
+ this.db = analyzer.getCatalog().getDbOrAnalysisException(dbName);
+
+ // check table
+ String tblName = this.dbTableName.getTbl();
+ if (Strings.isNullOrEmpty(tblName)) {
+ this.tables = this.db.getTables();
+ for (Table table : this.tables) {
+ checkAnalyzePriv(dbName, table.getName());
+ }
+ } else {
+ Table table = this.db.getTableOrAnalysisException(tblName);
+ this.tables = Collections.singletonList(table);
+ checkAnalyzePriv(dbName, table.getName());
+ }
+
+ // check column
+ if (this.columnNames == null || this.columnNames.isEmpty()) {
+ setTableIdToColumnName();
+ } else {
+ Table table = this.db.getTableOrAnalysisException(tblName);
+ for (String columnName : this.columnNames) {
+ Column column = table.getColumn(columnName);
+ if (column == null) {
+ ErrorReport.reportAnalysisException(ErrorCode.ERR_WRONG_COLUMN_NAME, columnName);
+ }
+ }
+ this.tableIdToColumnName.put(table.getId(), this.columnNames);
+ }
+ } else {
+ // analyze the current default db
+ String dbName = analyzer.getDefaultDb();
+ if (Strings.isNullOrEmpty(dbName)) {
+ ErrorReport.reportAnalysisException(ErrorCode.ERR_NO_DB_ERROR);
+ }
+ this.db = analyzer.getCatalog().getDbOrAnalysisException(dbName);
+ this.tables = this.db.getTables();
+ for (Table table : this.tables) {
+ checkAnalyzePriv(dbName, table.getName());
+ }
+ setTableIdToColumnName();
+ }
+
+ // step2: analyze properties
+ if (this.properties != null) {
+ for (Map.Entry<String, String> pros : this.properties.entrySet()) {
+ if (!"cbo_statistics_task_timeout_sec".equals(pros.getKey())) {
+ throw new AnalysisException("Unsupported property: " + pros.getKey());
+ }
+ if (!StringUtils.isNumeric(pros.getValue()) || Integer.parseInt(pros.getValue()) <= 0) {
+ throw new AnalysisException("Invalid property value: " + pros.getValue());
+ }
+ }
+ } else {
+ this.properties = Maps.newHashMap();
Review Comment:
Please init property map in the constructor.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticsJob.java:
##########
@@ -19,61 +19,151 @@
import org.apache.doris.analysis.AnalyzeStmt;
-import com.google.common.collect.Maps;
+import com.clearspring.analytics.util.Lists;
Review Comment:
error import
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyzeStmt.java:
##########
@@ -17,19 +17,181 @@
package org.apache.doris.analysis;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Column;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.cluster.ClusterNamespace;
+import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.Config;
+import org.apache.doris.common.ErrorCode;
+import org.apache.doris.common.ErrorReport;
+import org.apache.doris.common.UserException;
+import org.apache.doris.mysql.privilege.PaloAuth;
+import org.apache.doris.mysql.privilege.PrivPredicate;
+import org.apache.doris.qe.ConnectContext;
+
+import com.clearspring.analytics.util.Lists;
+import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
+import com.google.common.collect.Maps;
+
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
/**
* Collect statistics about a database
- *
+ * <p>
* syntax:
* ANALYZE [[ db_name.tb_name ] [( column_name [, ...] )], ...] [ PROPERTIES(...) ]
- *
- * db_name.tb_name: collect table and column statistics from tb_name
- *
- * column_name: collect column statistics from column_name
- *
- * properties: properties of statistics jobs
- *
+ * <p>
+ * db_name.tb_name: collect table and column statistics from tb_name
+ * <p>
+ * column_name: collect column statistics from column_name
+ * <p>
+ * properties: properties of statistics jobs
*/
public class AnalyzeStmt extends DdlStmt {
+ private final TableName dbTableName;
+ private final List<String> columnNames;
+ private Map<String, String> properties;
+
+ // after analyzed
+ private Database db;
+ private List<Table> tables;
+ private final Map<Long, List<String>> tableIdToColumnName = Maps.newHashMap();
+
+ public AnalyzeStmt(TableName dbTableName, List<String> columns, Map<String, String> properties) {
+ this.dbTableName = dbTableName;
+ this.columnNames = columns;
+ this.properties = properties;
+ }
+
+ public Database getDb() {
+ Preconditions.checkArgument(isAnalyzed(),
+ "The db name must be obtained after the parsing is complete");
+ return this.db;
+ }
+
+ public List<Table> getTables() {
+ Preconditions.checkArgument(isAnalyzed(),
+ "The db name must be obtained after the parsing is complete");
+ return this.tables;
+ }
+
+ public Map<Long, List<String>> getTableIdToColumnName() {
+ Preconditions.checkArgument(isAnalyzed(),
+ "The db name must be obtained after the parsing is complete");
+ return this.tableIdToColumnName;
+ }
+
+ public Map<String, String> getProperties() {
+ return this.properties;
+ }
+ @Override
+ public void analyze(Analyzer analyzer) throws UserException {
+ super.analyze(analyzer);
+
+ // step1: analyze database and table
+ if (this.dbTableName != null) {
+ String dbName;
+ if (Strings.isNullOrEmpty(this.dbTableName.getDb())) {
+ dbName = analyzer.getDefaultDb();
+ } else {
+ dbName = ClusterNamespace.getFullName(analyzer.getClusterName(), this.dbTableName.getDb());
+ }
+
+ // check db
+ if (Strings.isNullOrEmpty(dbName)) {
+ ErrorReport.reportAnalysisException(ErrorCode.ERR_NO_DB_ERROR);
+ }
+ this.db = analyzer.getCatalog().getDbOrAnalysisException(dbName);
+
+ // check table
+ String tblName = this.dbTableName.getTbl();
+ if (Strings.isNullOrEmpty(tblName)) {
+ this.tables = this.db.getTables();
+ for (Table table : this.tables) {
+ checkAnalyzePriv(dbName, table.getName());
+ }
+ } else {
+ Table table = this.db.getTableOrAnalysisException(tblName);
+ this.tables = Collections.singletonList(table);
+ checkAnalyzePriv(dbName, table.getName());
+ }
+
+ // check column
+ if (this.columnNames == null || this.columnNames.isEmpty()) {
+ setTableIdToColumnName();
+ } else {
+ Table table = this.db.getTableOrAnalysisException(tblName);
+ for (String columnName : this.columnNames) {
+ Column column = table.getColumn(columnName);
+ if (column == null) {
+ ErrorReport.reportAnalysisException(ErrorCode.ERR_WRONG_COLUMN_NAME, columnName);
+ }
+ }
+ this.tableIdToColumnName.put(table.getId(), this.columnNames);
+ }
+ } else {
+ // analyze the current default db
+ String dbName = analyzer.getDefaultDb();
+ if (Strings.isNullOrEmpty(dbName)) {
+ ErrorReport.reportAnalysisException(ErrorCode.ERR_NO_DB_ERROR);
+ }
+ this.db = analyzer.getCatalog().getDbOrAnalysisException(dbName);
+ this.tables = this.db.getTables();
+ for (Table table : this.tables) {
+ checkAnalyzePriv(dbName, table.getName());
+ }
+ setTableIdToColumnName();
+ }
+
+ // step2: analyze properties
+ if (this.properties != null) {
+ for (Map.Entry<String, String> pros : this.properties.entrySet()) {
+ if (!"cbo_statistics_task_timeout_sec".equals(pros.getKey())) {
Review Comment:
Please declare acceptable property set
For code examples see 'CreateRoutineLoadStmt'
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyzeStmt.java:
##########
@@ -17,19 +17,181 @@
package org.apache.doris.analysis;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Column;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.cluster.ClusterNamespace;
+import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.Config;
+import org.apache.doris.common.ErrorCode;
+import org.apache.doris.common.ErrorReport;
+import org.apache.doris.common.UserException;
+import org.apache.doris.mysql.privilege.PaloAuth;
+import org.apache.doris.mysql.privilege.PrivPredicate;
+import org.apache.doris.qe.ConnectContext;
+
+import com.clearspring.analytics.util.Lists;
+import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
+import com.google.common.collect.Maps;
+
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
/**
* Collect statistics about a database
- *
+ * <p>
* syntax:
* ANALYZE [[ db_name.tb_name ] [( column_name [, ...] )], ...] [ PROPERTIES(...) ]
- *
- * db_name.tb_name: collect table and column statistics from tb_name
- *
- * column_name: collect column statistics from column_name
- *
- * properties: properties of statistics jobs
- *
+ * <p>
+ * db_name.tb_name: collect table and column statistics from tb_name
+ * <p>
+ * column_name: collect column statistics from column_name
+ * <p>
+ * properties: properties of statistics jobs
*/
public class AnalyzeStmt extends DdlStmt {
+ private final TableName dbTableName;
+ private final List<String> columnNames;
+ private Map<String, String> properties;
+
+ // after analyzed
+ private Database db;
Review Comment:
It is better to only save id in here
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticsJobManager.java:
##########
@@ -27,47 +35,105 @@
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
-/*
-For unified management of statistics job,
-including job addition, cancellation, scheduling, etc.
+/**
+ * For unified management of statistics job,
+ * including job addition, cancellation, scheduling, etc.
*/
public class StatisticsJobManager {
private static final Logger LOG = LogManager.getLogger(StatisticsJobManager.class);
- // statistics job
- private Map<Long, StatisticsJob> idToStatisticsJob = Maps.newConcurrentMap();
+ /**
+ * save statistics job status information
+ */
+ private final Map<Long, StatisticsJob> idToStatisticsJob = Maps.newConcurrentMap();
- public void createStatisticsJob(AnalyzeStmt analyzeStmt) {
- // step0: init statistics job by analyzeStmt
+ private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(true);
+
+ public void readLock() {
+ lock.readLock().lock();
+ }
+
+ public void readUnlock() {
+ lock.readLock().unlock();
+ }
+
+ private void writeLock() {
+ lock.writeLock().lock();
+ }
+
+ private void writeUnlock() {
+ lock.writeLock().unlock();
+ }
+
+ public Map<Long, StatisticsJob> getIdToStatisticsJob() {
+ return this.idToStatisticsJob;
+ }
+
+ public void createStatisticsJob(AnalyzeStmt analyzeStmt) throws UserException {
+ // step1: init statistics job by analyzeStmt
StatisticsJob statisticsJob = StatisticsJob.fromAnalyzeStmt(analyzeStmt);
- // step1: get statistics to be analyzed
- Set<Long> tableIdList = statisticsJob.relatedTableId();
- // step2: check restrict
- checkRestrict(tableIdList);
- // step3: check permission
- checkPermission();
- // step4: create it
- createStatisticsJob(statisticsJob);
+ writeLock();
+ try {
+ // step2: check restrict
+ this.checkRestrict(analyzeStmt.getDb(), statisticsJob.getTblIds());
+ // step3: create it
+ this.createStatisticsJob(statisticsJob);
+ } finally {
+ writeUnlock();
+ }
}
- public void createStatisticsJob(StatisticsJob statisticsJob) {
- idToStatisticsJob.put(statisticsJob.getId(), statisticsJob);
+ public void createStatisticsJob(StatisticsJob statisticsJob) throws DdlException {
+ // assign the id when the job is ready to run
+ statisticsJob.setId(Catalog.getCurrentCatalog().getNextId());
+ this.idToStatisticsJob.put(statisticsJob.getId(), statisticsJob);
try {
Catalog.getCurrentCatalog().getStatisticsJobScheduler().addPendingJob(statisticsJob);
} catch (IllegalStateException e) {
LOG.info("The pending statistics job is full. Please submit it again later.");
+ throw new DdlException("The pending statistics job is full, Please submit it again later.");
}
}
- // Rule1: The same table cannot have two unfinished statistics jobs
- // Rule2: The unfinished statistics job could not more then Config.max_statistics_job_num
- // Rule3: The job for external table is not supported
- private void checkRestrict(Set<Long> tableIdList) {
- // TODO
- }
+ /**
+ * The statistical job has the following restrict:
+ * - Rule1: The same table cannot have two unfinished statistics jobs
+ * - Rule2: The unfinished statistics job could not more then Config.max_statistics_job_num
+ * - Rule3: The job for external table is not supported
+ */
+ private void checkRestrict(Database db, Set<Long> tableIds) throws AnalysisException {
+ // check table type
+ for (Long tableId : tableIds) {
Review Comment:
It is better to use dbId and double get db in here.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticsJob.java:
##########
@@ -19,61 +19,151 @@
import org.apache.doris.analysis.AnalyzeStmt;
-import com.google.common.collect.Maps;
+import com.clearspring.analytics.util.Lists;
-import org.glassfish.jersey.internal.guava.Sets;
+import org.apache.log4j.LogManager;
+import org.apache.log4j.Logger;
import java.util.List;
import java.util.Map;
import java.util.Set;
-import com.clearspring.analytics.util.Lists;
-
-/*
-Used to store statistics job info,
-including job status, progress, etc.
+/***
+ * Used to store statistics job info,
+ * including job status, progress, etc.
*/
public class StatisticsJob {
+ private static final Logger LOG = LogManager.getLogger(StatisticsJob.class);
public enum JobState {
PENDING,
SCHEDULING,
RUNNING,
FINISHED,
- CANCELLED
+ CANCELLED,
+ FAILED
}
private long id = -1;
Review Comment:
```suggestion
private long id = Catalog.getCurrentCatalog().getNextId();
```
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticsJobManager.java:
##########
@@ -27,47 +35,105 @@
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
-/*
-For unified management of statistics job,
-including job addition, cancellation, scheduling, etc.
+/**
+ * For unified management of statistics job,
+ * including job addition, cancellation, scheduling, etc.
*/
public class StatisticsJobManager {
private static final Logger LOG = LogManager.getLogger(StatisticsJobManager.class);
- // statistics job
- private Map<Long, StatisticsJob> idToStatisticsJob = Maps.newConcurrentMap();
+ /**
+ * save statistics job status information
+ */
+ private final Map<Long, StatisticsJob> idToStatisticsJob = Maps.newConcurrentMap();
- public void createStatisticsJob(AnalyzeStmt analyzeStmt) {
- // step0: init statistics job by analyzeStmt
+ private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(true);
+
+ public void readLock() {
+ lock.readLock().lock();
+ }
+
+ public void readUnlock() {
+ lock.readLock().unlock();
+ }
+
+ private void writeLock() {
+ lock.writeLock().lock();
+ }
+
+ private void writeUnlock() {
+ lock.writeLock().unlock();
+ }
+
+ public Map<Long, StatisticsJob> getIdToStatisticsJob() {
+ return this.idToStatisticsJob;
+ }
+
+ public void createStatisticsJob(AnalyzeStmt analyzeStmt) throws UserException {
+ // step1: init statistics job by analyzeStmt
StatisticsJob statisticsJob = StatisticsJob.fromAnalyzeStmt(analyzeStmt);
- // step1: get statistics to be analyzed
- Set<Long> tableIdList = statisticsJob.relatedTableId();
- // step2: check restrict
- checkRestrict(tableIdList);
- // step3: check permission
- checkPermission();
- // step4: create it
- createStatisticsJob(statisticsJob);
+ writeLock();
+ try {
+ // step2: check restrict
+ this.checkRestrict(analyzeStmt.getDb(), statisticsJob.getTblIds());
+ // step3: create it
+ this.createStatisticsJob(statisticsJob);
+ } finally {
+ writeUnlock();
+ }
}
- public void createStatisticsJob(StatisticsJob statisticsJob) {
- idToStatisticsJob.put(statisticsJob.getId(), statisticsJob);
+ public void createStatisticsJob(StatisticsJob statisticsJob) throws DdlException {
+ // assign the id when the job is ready to run
+ statisticsJob.setId(Catalog.getCurrentCatalog().getNextId());
+ this.idToStatisticsJob.put(statisticsJob.getId(), statisticsJob);
try {
Catalog.getCurrentCatalog().getStatisticsJobScheduler().addPendingJob(statisticsJob);
} catch (IllegalStateException e) {
LOG.info("The pending statistics job is full. Please submit it again later.");
+ throw new DdlException("The pending statistics job is full, Please submit it again later.");
}
}
- // Rule1: The same table cannot have two unfinished statistics jobs
- // Rule2: The unfinished statistics job could not more then Config.max_statistics_job_num
- // Rule3: The job for external table is not supported
- private void checkRestrict(Set<Long> tableIdList) {
- // TODO
- }
+ /**
+ * The statistical job has the following restrict:
+ * - Rule1: The same table cannot have two unfinished statistics jobs
+ * - Rule2: The unfinished statistics job could not more then Config.max_statistics_job_num
+ * - Rule3: The job for external table is not supported
+ */
+ private void checkRestrict(Database db, Set<Long> tableIds) throws AnalysisException {
+ // check table type
+ for (Long tableId : tableIds) {
+ Table table = db.getTableOrAnalysisException(tableId);
+ if (table.getType() != Table.TableType.OLAP) {
+ ErrorReport.reportAnalysisException(ErrorCode.ERR_NOT_OLAP_TABLE, db.getFullName(), table.getName(), "ANALYZE");
+ }
+ }
- private void checkPermission() {
- // TODO
+ int unfinishedJobs = 0;
+
+ // check table unfinished job
+ for (StatisticsJob statisticsJob : this.idToStatisticsJob.values()) {
+ StatisticsJob.JobState jobState = statisticsJob.getJobState();
+ Set<Long> tblIds = statisticsJob.getTblIds();
+ if (jobState == StatisticsJob.JobState.PENDING
+ || jobState == StatisticsJob.JobState.SCHEDULING
+ || jobState == StatisticsJob.JobState.RUNNING) {
+ for (Long tableId : tableIds) {
+ if (tblIds.contains(tableId)) {
Review Comment:
Or save a Map<TableId, StatisticsJob> ?
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticsJobManager.java:
##########
@@ -27,47 +35,105 @@
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
-/*
-For unified management of statistics job,
-including job addition, cancellation, scheduling, etc.
+/**
+ * For unified management of statistics job,
+ * including job addition, cancellation, scheduling, etc.
*/
public class StatisticsJobManager {
private static final Logger LOG = LogManager.getLogger(StatisticsJobManager.class);
- // statistics job
- private Map<Long, StatisticsJob> idToStatisticsJob = Maps.newConcurrentMap();
+ /**
+ * save statistics job status information
+ */
+ private final Map<Long, StatisticsJob> idToStatisticsJob = Maps.newConcurrentMap();
- public void createStatisticsJob(AnalyzeStmt analyzeStmt) {
- // step0: init statistics job by analyzeStmt
+ private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(true);
+
+ public void readLock() {
+ lock.readLock().lock();
+ }
+
+ public void readUnlock() {
+ lock.readLock().unlock();
+ }
+
+ private void writeLock() {
+ lock.writeLock().lock();
+ }
+
+ private void writeUnlock() {
+ lock.writeLock().unlock();
+ }
+
+ public Map<Long, StatisticsJob> getIdToStatisticsJob() {
+ return this.idToStatisticsJob;
+ }
+
+ public void createStatisticsJob(AnalyzeStmt analyzeStmt) throws UserException {
+ // step1: init statistics job by analyzeStmt
StatisticsJob statisticsJob = StatisticsJob.fromAnalyzeStmt(analyzeStmt);
- // step1: get statistics to be analyzed
- Set<Long> tableIdList = statisticsJob.relatedTableId();
- // step2: check restrict
- checkRestrict(tableIdList);
- // step3: check permission
- checkPermission();
- // step4: create it
- createStatisticsJob(statisticsJob);
+ writeLock();
+ try {
+ // step2: check restrict
+ this.checkRestrict(analyzeStmt.getDb(), statisticsJob.getTblIds());
+ // step3: create it
+ this.createStatisticsJob(statisticsJob);
+ } finally {
+ writeUnlock();
+ }
}
- public void createStatisticsJob(StatisticsJob statisticsJob) {
- idToStatisticsJob.put(statisticsJob.getId(), statisticsJob);
+ public void createStatisticsJob(StatisticsJob statisticsJob) throws DdlException {
+ // assign the id when the job is ready to run
+ statisticsJob.setId(Catalog.getCurrentCatalog().getNextId());
Review Comment:
remove it
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org