[AMORO-3747] Introducing a more general TableRuntime storage structure#3748
[AMORO-3747] Introducing a more general TableRuntime storage structure#3748baiyangtx merged 2 commits intoapache:masterfrom
Conversation
baiyangtx
force-pushed
the
new_table_runtime2
branch
from
b84230e to
db20fc4
Compare
| private class TableRuntimeOperationImpl implements TableRuntimeOperation { | ||
|
|
||
| private final TableRuntimeMeta oldMeta; | ||
| private final List<Runnable> opeartions = Lists.newArrayList(); |
There was a problem hiding this comment.
nit
| private final List<Runnable> opeartions = Lists.newArrayList(); | |
| private final List<Runnable> operations = Lists.newArrayList(); |
There was a problem hiding this comment.
tks, I have fix it.
| @@ -685,11 +686,6 @@ public void cancelOptimizingProcess(Context ctx) { | |||
| ServerTableIdentifier serverTableIdentifier = | |||
There was a problem hiding this comment.
This can be also removed if don't check the processId
There was a problem hiding this comment.
I have add check logic.
| } | ||
|
|
||
| @Override | ||
| public TableFormat getFormat() { |
There was a problem hiding this comment.
getFormat already has default impelementation
There was a problem hiding this comment.
I have remove the implement.
|
|
||
| @Override | ||
| public AmoroProcess<? extends TableProcessState> trigger(Action action) { | ||
| return null; |
There was a problem hiding this comment.
Better to throw an UnsupportException or make it abstract
There was a problem hiding this comment.
I have make it abstract
amoro-ams/src/main/java/org/apache/amoro/server/table/AbstractTableRuntime.java
Outdated
Show resolved
Hide resolved
amoro-ams/src/main/java/org/apache/amoro/server/table/AbstractTableRuntime.java
Outdated
Show resolved
Hide resolved
| `pending_input` mediumtext, | ||
| `table_summary` mediumtext, | ||
| `group_name` varchar(64) NOT NULL, | ||
| `status_code` int DEFAULT 0 NOT NULL COMMENT 'Table runtime status code.', |
There was a problem hiding this comment.
0 status_code makes no sense, do you mean 700(IDLE)?
baiyangtx
force-pushed
the
new_table_runtime2
branch
from
1976d18 to
a859a96
Compare
zhoujinsong
left a comment
zhoujinsong
left a comment
There was a problem hiding this comment.
This change looks good to me.
But this PR makes a lot change to some core codes.
I would give more time to other developers to review.
| import java.util.function.BiFunction; | ||
| import java.util.function.Function; | ||
|
|
||
| public class StateKey<T> { |
There was a problem hiding this comment.
Maybe add some comments for these new classes/interfaces?
Aireed
left a comment
Aireed
left a comment
There was a problem hiding this comment.
LGTM.
Wait for others to take a look.
4 participants
Why are the changes needed?
Close #3747.
Brief change log
table_runtime_stateto store extensiable informationHow 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