You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@amoro.apache.org by "baiyangtx (via GitHub)" <gi...@apache.org> on 2025/08/06 16:07:11 UTC
[PR] [Improvement] Refactor table_optimizing_process into a more generic structure [amoro]
baiyangtx opened a new pull request, #3715:
URL: https://github.com/apache/amoro/pull/3715
## Why are the changes needed?
Try to inturduce a more generic structure of TableProcess, and to support more execution engine such as Spark or FlinkSQL
## Brief change log
- Split table_optimizing_process into table_process and internal_table_process_state to table.
- Refactor related codes
## How was this patch tested?
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [ ] Run test locally before making a pull request
## Documentation
- Does this pull request introduce a new feature? (no)
--
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@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Refactor table_optimizing_process into a more generic structure [amoro]
Posted by "cxxiii (via GitHub)" <gi...@apache.org>.
cxxiii commented on code in PR #3715:
URL: https://github.com/apache/amoro/pull/3715#discussion_r2265983689
##########
amoro-ams/src/main/resources/mysql/ams-mysql-init.sql:
##########
@@ -135,26 +135,33 @@ CREATE TABLE `table_runtime`
INDEX idx_optimizer_status_and_time (optimizing_status_code, optimizing_status_start_time DESC)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COMMENT 'Optimize running information of each table' ROW_FORMAT=DYNAMIC;
-CREATE TABLE `table_optimizing_process`
+CREATE TABLE `table_process`
+(
+ `process_id` bigint(20) NOT NULL COMMENT 'table process id',
+ `table_id` bigint(20) NOT NULL COMMENT 'table id',
+ `status` varchar(64) NOT NULL COMMENT 'Table optimizing status',
+ `process_type` varchar(64) NOT NULL COMMENT 'Process action type',
+ `process_stage` varchar(64) NOT NULL COMMENT 'Process current stage',
+ `execution_engine` varchar(64) NOT NULL COMMENT 'Execution engine',
+ `create_time` timestamp DEFAULT CURRENT_TIMESTAMP COMMENT 'First plan time',
+ `finish_time` timestamp NULL DEFAULT NULL COMMENT 'finish time or failed time',
+ `fail_message` mediumtext(4096) DEFAULT NULL COMMENT 'Error message after task failed',
+ `summary` mediumtext COMMENT 'Max change transaction id of these tasks',
+ PRIMARY KEY (`process_id`),
+ KEY `table_index` (`table_id`, `create_time`)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COMMENT 'History of optimizing after each commit';
+
+CREATE TABLE `optimizing_process_state`
(
`process_id` bigint(20) NOT NULL COMMENT 'optimizing_procedure UUID',
`table_id` bigint(20) NOT NULL,
- `catalog_name` varchar(64) NOT NULL COMMENT 'Catalog name',
- `db_name` varchar(128) NOT NULL COMMENT 'Database name',
- `table_name` varchar(256) NOT NULL COMMENT 'Table name',
`target_snapshot_id` bigint(20) NOT NULL,
`target_change_snapshot_id` bigint(20) NOT NULL,
- `status` varchar(10) NOT NULL COMMENT 'Direct to TableOptimizingStatus',
- `optimizing_type` varchar(10) NOT NULL COMMENT 'Optimize type: Major, Minor',
- `plan_time` timestamp DEFAULT CURRENT_TIMESTAMP COMMENT 'First plan time',
- `end_time` timestamp NULL DEFAULT NULL COMMENT 'finish time or failed time',
- `fail_reason` varchar(4096) DEFAULT NULL COMMENT 'Error message after task failed',
`rewrite_input` longblob DEFAULT NULL COMMENT 'rewrite files input',
- `summary` mediumtext COMMENT 'Max change transaction id of these tasks',
`from_sequence` mediumtext COMMENT 'from or min sequence of each partition',
`to_sequence` mediumtext COMMENT 'to or max sequence of each partition',
- PRIMARY KEY (`process_id`),
- KEY `table_index` (`table_id`, `plan_time`)
+ PRIMARY KEY (`process_id`)
Review Comment:
Hello, it seems that a comma is missing 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.
To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Refactor table_optimizing_process into a more generic structure [amoro]
Posted by "Aireed (via GitHub)" <gi...@apache.org>.
Aireed commented on code in PR #3715:
URL: https://github.com/apache/amoro/pull/3715#discussion_r2261832033
##########
amoro-ams/src/test/java/org/apache/amoro/server/dashboard/TestIcebergServerTableDescriptor.java:
##########
@@ -306,17 +307,25 @@ public void insertOptimizingProcess(
Map<String, Long> fromSequence,
Map<String, Long> toSequence) {
doAs(
- OptimizingMapper.class,
+ TableProcessMapper.class,
mapper ->
- mapper.insertOptimizingProcess(
- identifier,
+ mapper.insertProcess(
+ identifier.getId(),
processId,
- targetSnapshotId,
- targetChangeSnapshotId,
status,
- type,
+ type.name(),
Review Comment:
OptimizingType.name is upperCase, OptimizingStatus.name is lowercase，
so we need keep consistence with other` insertProcess` call(like `OptimizingQueue.java::beginAndPersistProcess`) which use `OptimizingType.name` and `OptimizingStatus.name`
--
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@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Refactor table_optimizing_process into a more generic structure [amoro]
Posted by "xxubai (via GitHub)" <gi...@apache.org>.
xxubai commented on code in PR #3715:
URL: https://github.com/apache/amoro/pull/3715#discussion_r2268723620
##########
amoro-ams/src/main/resources/derby/ams-derby-init.sql:
##########
@@ -123,25 +123,32 @@ CREATE TABLE table_runtime (
CONSTRAINT table_runtime_table_name_idx UNIQUE (catalog_name, db_name, table_name)
);
-CREATE TABLE table_optimizing_process (
- process_id BIGINT NOT NULL,
- table_id BIGINT NOT NULL,
- catalog_name VARCHAR(64) NOT NULL,
- db_name VARCHAR(128) NOT NULL,
- table_name VARCHAR(256) NOT NULL,
- target_snapshot_id BIGINT NOT NULL,
- target_change_snapshot_id BIGINT NOT NULL,
- status VARCHAR(10) NOT NULL,
- optimizing_type VARCHAR(10) NOT NULL,
- plan_time TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
- end_time TIMESTAMP DEFAULT NULL,
- fail_reason VARCHAR(4096),
- rewrite_input BLOB(64m),
- summary CLOB(64m),
- from_sequence CLOB(64m),
- to_sequence CLOB(64m),
- CONSTRAINT table_optimizing_process_pk PRIMARY KEY (process_id)
+CREATE TABLE table_process (
+ process_id BIGINT NOT NULL PRIMARY KEY,
+ table_id BIGINT NOT NULL,
+ status VARCHAR(64) NOT NULL,
+ process_type VARCHAR(64) NOT NULL,
+ process_stage VARCHAR(64) NOT NULL,
+ execution_engine VARCHAR(64) NOT NULL,
+ create_time TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
+ finish_time TIMESTAMP DEFAULT NULL,
+ fail_message CLOB,
+ summary CLOB
+);
+CREATE INDEX table_process_table_idx ON table_process (table_id, create_time);
+
+CREATE TABLE optimizing_process_state (
+ process_id BIGINT NOT NULL PRIMARY KEY,
+ table_id BIGINT NOT NULL,
+ target_snapshot_id BIGINT NOT NULL,
+ target_change_snapshot_id BIGINT NOT NULL,
+ rewrite_input BLOB,
Review Comment:
Keep the maximum size the same as the original size.
--
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@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Refactor table_optimizing_process into a more generic structure [amoro]
Posted by "baiyangtx (via GitHub)" <gi...@apache.org>.
baiyangtx merged PR #3715:
URL: https://github.com/apache/amoro/pull/3715
--
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@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Refactor table_optimizing_process into a more generic structure [amoro]
Posted by "xxubai (via GitHub)" <gi...@apache.org>.
xxubai commented on code in PR #3715:
URL: https://github.com/apache/amoro/pull/3715#discussion_r2262362833
##########
amoro-ams/src/test/java/org/apache/amoro/server/dashboard/TestIcebergServerTableDescriptor.java:
##########
@@ -306,17 +307,25 @@ public void insertOptimizingProcess(
Map<String, Long> fromSequence,
Map<String, Long> toSequence) {
doAs(
- OptimizingMapper.class,
+ TableProcessMapper.class,
mapper ->
- mapper.insertOptimizingProcess(
- identifier,
+ mapper.insertProcess(
+ identifier.getId(),
processId,
- targetSnapshotId,
- targetChangeSnapshotId,
status,
- type,
+ type.name(),
+ type.name(),
+ "AMORO",
Review Comment:
Why use fixed execution engine type `AMORO` when inserting process record
--
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@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Refactor table_optimizing_process into a more generic structure [amoro]
Posted by "baiyangtx (via GitHub)" <gi...@apache.org>.
baiyangtx commented on code in PR #3715:
URL: https://github.com/apache/amoro/pull/3715#discussion_r2268449008
##########
amoro-ams/src/test/java/org/apache/amoro/server/dashboard/TestIcebergServerTableDescriptor.java:
##########
@@ -306,17 +307,25 @@ public void insertOptimizingProcess(
Map<String, Long> fromSequence,
Map<String, Long> toSequence) {
doAs(
- OptimizingMapper.class,
+ TableProcessMapper.class,
mapper ->
- mapper.insertOptimizingProcess(
- identifier,
+ mapper.insertProcess(
+ identifier.getId(),
processId,
- targetSnapshotId,
- targetChangeSnapshotId,
status,
- type,
+ type.name(),
+ type.name(),
+ "AMORO",
Review Comment:
In the future, we will support other execution engines like 'Spark'
--
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@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org