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 2020/01/14 12:14:08 UTC
[GitHub] [incubator-doris] WingsGo opened a new pull request #2763:
[Alter]Add wait time while doing alter options
WingsGo opened a new pull request #2763: [Alter]Add wait time while doing alter options
URL: https://github.com/apache/incubator-doris/pull/2763
#2637
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman merged pull request #2763: [Alter]Add
wait time while doing alter options
Posted by GitBox <gi...@apache.org>.
morningman merged pull request #2763: [Alter]Add wait time while doing alter options
URL: https://github.com/apache/incubator-doris/pull/2763
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request
#2763: [Alter]Add wait time while doing alter options
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2763: [Alter]Add wait time while doing alter options
URL: https://github.com/apache/incubator-doris/pull/2763#discussion_r367437783
##########
File path: fe/src/main/java/org/apache/doris/clone/TabletChecker.java
##########
@@ -356,71 +368,72 @@ private void removePriosIfNecessary() {
* when being scheduled.
*/
public void repairTable(AdminRepairTableStmt stmt) throws DdlException {
- Catalog catalog = Catalog.getCurrentCatalog();
- Database db = catalog.getDb(stmt.getDbName());
- if (db == null) {
- throw new DdlException("Database " + stmt.getDbName() + " does not exist");
- }
+ RepairTabletInfo repairTabletInfo = getRepairTabletInfo(stmt.getDbName(), stmt.getTblName(), stmt.getPartitions());
+ addPrios(repairTabletInfo.dbId, repairTabletInfo.tblId, repairTabletInfo.partIds, stmt.getTimeoutS() * 1000);
Review comment:
Why not just pass the entire `repairTabletInfo`? instead of each field of `repairTabletInfo`?
And `addPrios()` can be a private method.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request
#2763: [Alter]Add wait time while doing alter options
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2763: [Alter]Add wait time while doing alter options
URL: https://github.com/apache/incubator-doris/pull/2763#discussion_r367437908
##########
File path: fe/src/main/java/org/apache/doris/clone/TabletChecker.java
##########
@@ -356,71 +368,72 @@ private void removePriosIfNecessary() {
* when being scheduled.
*/
public void repairTable(AdminRepairTableStmt stmt) throws DdlException {
- Catalog catalog = Catalog.getCurrentCatalog();
- Database db = catalog.getDb(stmt.getDbName());
- if (db == null) {
- throw new DdlException("Database " + stmt.getDbName() + " does not exist");
- }
+ RepairTabletInfo repairTabletInfo = getRepairTabletInfo(stmt.getDbName(), stmt.getTblName(), stmt.getPartitions());
+ addPrios(repairTabletInfo.dbId, repairTabletInfo.tblId, repairTabletInfo.partIds, stmt.getTimeoutS() * 1000);
+ LOG.info("repair database: {}, table: {}, partition: {}", repairTabletInfo.dbId, repairTabletInfo.tblId, repairTabletInfo.partIds);
+ }
- long dbId = db.getId();
- long tblId = -1;
- List<Long> partIds = Lists.newArrayList();
- db.readLock();
- try {
- Table tbl = db.getTable(stmt.getTblName());
- if (tbl == null || tbl.getType() != TableType.OLAP) {
- throw new DdlException("Table does not exist or is not OLAP table: " + stmt.getTblName());
+ /*
+ * handle ADMIN CANCEL REPAIR TABLE stmt send by user.
+ * This operation will remove the specified partitions from 'prios'
+ */
+ public void cancelRepairTable(AdminCancelRepairTableStmt stmt) throws DdlException {
+ RepairTabletInfo repairTabletInfo = getRepairTabletInfo(stmt.getDbName(), stmt.getTblName(), stmt.getPartitions());
+ removePrios(repairTabletInfo.dbId, repairTabletInfo.tblId, repairTabletInfo.partIds);
Review comment:
```suggestion
removePrios(repairTabletInfo);
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request
#2763: [Alter]Add wait time while doing alter options
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2763: [Alter]Add wait time while doing alter options
URL: https://github.com/apache/incubator-doris/pull/2763#discussion_r368214827
##########
File path: fe/src/main/java/org/apache/doris/alter/RollupJobV2.java
##########
@@ -168,8 +168,17 @@ protected void runPendingJob() throws AlterCancelException {
if (tbl == null) {
throw new AlterCancelException("Table " + tableId + " does not exist");
}
- Preconditions.checkState(tbl.getState() == OlapTableState.ROLLUP);
+ boolean isStable = tbl.isStable(Catalog.getCurrentSystemInfo(),
+ Catalog.getCurrentCatalog().getTabletScheduler(),
+ db.getClusterName());
+ if (!isStable) {
+ errMsg = "table is unstable";
+ LOG.warn("doing rollup job: " + jobId + "while table is not stable.");
Review comment:
```suggestion
LOG.warn("doing rollup job: " + jobId + " while table is not stable.");
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] WingsGo commented on a change in pull request
#2763: [Alter]Add wait time while doing alter options
Posted by GitBox <gi...@apache.org>.
WingsGo commented on a change in pull request #2763: [Alter]Add wait time while doing alter options
URL: https://github.com/apache/incubator-doris/pull/2763#discussion_r367510314
##########
File path: fe/src/main/java/org/apache/doris/alter/Alter.java
##########
@@ -276,8 +276,9 @@ public void processAlterTable(AlterTableStmt stmt) throws UserException {
if (olapTable.getState() != OlapTableState.NORMAL) {
throw new DdlException("Table[" + table.getName() + "]'s state is not NORMAL. Do not allow doing ALTER ops");
}
-
- if (needTableStable) {
+
+ // schema change job will wait until table become stable
+ if (needTableStable && !hasSchemaChange) {
Review comment:
@morningman Done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request
#2763: [Alter]Add wait time while doing alter options
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2763: [Alter]Add wait time while doing alter options
URL: https://github.com/apache/incubator-doris/pull/2763#discussion_r367439828
##########
File path: fe/src/main/java/org/apache/doris/alter/Alter.java
##########
@@ -276,8 +276,9 @@ public void processAlterTable(AlterTableStmt stmt) throws UserException {
if (olapTable.getState() != OlapTableState.NORMAL) {
throw new DdlException("Table[" + table.getName() + "]'s state is not NORMAL. Do not allow doing ALTER ops");
}
-
- if (needTableStable) {
+
+ // schema change job will wait until table become stable
+ if (needTableStable && !hasSchemaChange) {
Review comment:
I think rollup job can be changed to same as schema change job.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request
#2763: [Alter]Add wait time while doing alter options
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2763: [Alter]Add wait time while doing alter options
URL: https://github.com/apache/incubator-doris/pull/2763#discussion_r368215051
##########
File path: fe/src/main/java/org/apache/doris/alter/SchemaChangeJobV2.java
##########
@@ -184,13 +184,23 @@ protected void runPendingJob() throws AlterCancelException {
totalReplicaNum += tablet.getReplicas().size();
}
}
- MarkedCountDownLatch<Long, Long> countDownLatch = new MarkedCountDownLatch<Long, Long>(totalReplicaNum);
+ MarkedCountDownLatch<Long, Long> countDownLatch = new MarkedCountDownLatch<>(totalReplicaNum);
db.readLock();
try {
OlapTable tbl = (OlapTable) db.getTable(tableId);
if (tbl == null) {
throw new AlterCancelException("Table " + tableId + " does not exist");
- }
+ }
+
+ boolean isStable = tbl.isStable(Catalog.getCurrentSystemInfo(),
+ Catalog.getCurrentCatalog().getTabletScheduler(),
+ db.getClusterName());
+ if (!isStable) {
+ errMsg = "table is unstable";
+ LOG.warn("doing schema change job: " + jobId + "while table is not stable.");
Review comment:
```suggestion
LOG.warn("doing schema change job: " + jobId + " while table is not stable.");
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request
#2763: [Alter]Add wait time while doing alter options
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2763: [Alter]Add wait time while doing alter options
URL: https://github.com/apache/incubator-doris/pull/2763#discussion_r366373998
##########
File path: fe/src/main/java/org/apache/doris/alter/Alter.java
##########
@@ -273,6 +276,18 @@ public void processAlterTable(AlterTableStmt stmt) throws UserException {
throw new DdlException("table with empty parition cannot do schema change. [" + tableName + "]");
}
+ // if table state is unhealthy, change table repair priority, and wait until repair finish or exceed timeout
+ if (olapTable.getState() != OlapTableState.NORMAL) {
Review comment:
First, `olapTable.getState() != OlapTableState.NORMAL` does not mean the table is unhealthy.
Second, it's not good to sleep here, it will block the client connection for a very long time.
Third, I don't think its good idea to add priority repair here. We should solve more why the table is often unhealthy, rather than increase the complexity here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] WingsGo commented on a change in pull request
#2763: [Alter]Add wait time while doing alter options
Posted by GitBox <gi...@apache.org>.
WingsGo commented on a change in pull request #2763: [Alter]Add wait time while doing alter options
URL: https://github.com/apache/incubator-doris/pull/2763#discussion_r366781873
##########
File path: fe/src/main/java/org/apache/doris/alter/Alter.java
##########
@@ -273,6 +276,18 @@ public void processAlterTable(AlterTableStmt stmt) throws UserException {
throw new DdlException("table with empty parition cannot do schema change. [" + tableName + "]");
}
+ // if table state is unhealthy, change table repair priority, and wait until repair finish or exceed timeout
+ if (olapTable.getState() != OlapTableState.NORMAL) {
Review comment:
@morningman Yes, you're right, I change the waiting logic to runPendingJob.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request
#2763: [Alter]Add wait time while doing alter options
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2763: [Alter]Add wait time while doing alter options
URL: https://github.com/apache/incubator-doris/pull/2763#discussion_r367436403
##########
File path: fe/src/main/java/org/apache/doris/alter/SchemaChangeJobV2.java
##########
@@ -184,13 +184,22 @@ protected void runPendingJob() throws AlterCancelException {
totalReplicaNum += tablet.getReplicas().size();
}
}
- MarkedCountDownLatch<Long, Long> countDownLatch = new MarkedCountDownLatch<Long, Long>(totalReplicaNum);
+ MarkedCountDownLatch<Long, Long> countDownLatch = new MarkedCountDownLatch<>(totalReplicaNum);
db.readLock();
try {
OlapTable tbl = (OlapTable) db.getTable(tableId);
if (tbl == null) {
throw new AlterCancelException("Table " + tableId + " does not exist");
- }
+ }
+
+ boolean isStable = tbl.isStable(Catalog.getCurrentSystemInfo(),
+ Catalog.getCurrentCatalog().getTabletScheduler(),
+ db.getClusterName());
+ if (!isStable) {
+ LOG.warn("doing schema change job while table is not stable, wait until tablet to be repaired.");
Review comment:
You can add this error msg to the `errMsg` of the AlterJob, so that user can see what happen.
And this log should contains job id for debugging. And in `errMsg`, it can be shorten, eg, "table is unstable".
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org