-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[core] Support RollbackAction and Procedure with timestamp #4333
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,10 @@ | |
|
||
package org.apache.paimon.flink.action; | ||
|
||
import org.apache.paimon.Snapshot; | ||
import org.apache.paimon.table.DataTable; | ||
import org.apache.paimon.table.FileStoreTable; | ||
import org.apache.paimon.utils.Preconditions; | ||
|
||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
@@ -32,14 +35,18 @@ public class RollbackToAction extends TableActionBase { | |
|
||
private final String version; | ||
|
||
private final Boolean isTimestamp; | ||
|
||
public RollbackToAction( | ||
String warehouse, | ||
String databaseName, | ||
String tableName, | ||
String version, | ||
Boolean isTimestamp, | ||
Map<String, String> catalogConfig) { | ||
super(warehouse, databaseName, tableName, catalogConfig); | ||
this.version = version; | ||
this.isTimestamp = isTimestamp; | ||
} | ||
|
||
@Override | ||
|
@@ -50,8 +57,20 @@ public void run() throws Exception { | |
throw new IllegalArgumentException("Unknown table: " + identifier); | ||
} | ||
|
||
FileStoreTable fileStoreTable = (FileStoreTable) table; | ||
if (version.chars().allMatch(Character::isDigit)) { | ||
table.rollbackTo(Long.parseLong(version)); | ||
if (isTimestamp != null && isTimestamp) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. boolean There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here may not be boolean, because this argument may be null. |
||
Snapshot snapshot = | ||
fileStoreTable | ||
.snapshotManager() | ||
.earlierOrEqualTimeMills(Long.parseLong(version)); | ||
Preconditions.checkNotNull( | ||
snapshot, | ||
String.format("count not find snapshot earlier than %s", version)); | ||
fileStoreTable.rollbackTo(snapshot.id()); | ||
} else { | ||
table.rollbackTo(Long.parseLong(version)); | ||
} | ||
} else { | ||
table.rollbackTo(version); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,8 @@ public class RollbackToActionFactory implements ActionFactory { | |
|
||
private static final String VERSION = "version"; | ||
|
||
private static final String IS_TIMESTAMP = "is_timestamp"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should also modify doc. |
||
|
||
@Override | ||
public String identifier() { | ||
return IDENTIFIER; | ||
|
@@ -41,12 +43,18 @@ public Optional<Action> create(MultipleParameterToolAdapter params) { | |
|
||
checkRequiredArgument(params, VERSION); | ||
String version = params.get(VERSION); | ||
Boolean isTimestamp = Boolean.parseBoolean(params.get(IS_TIMESTAMP)); | ||
|
||
Map<String, String> catalogConfig = optionalConfigMap(params, CATALOG_CONF); | ||
|
||
RollbackToAction action = | ||
new RollbackToAction( | ||
tablePath.f0, tablePath.f1, tablePath.f2, version, catalogConfig); | ||
tablePath.f0, | ||
tablePath.f1, | ||
tablePath.f2, | ||
version, | ||
isTimestamp, | ||
catalogConfig); | ||
|
||
return Optional.of(action); | ||
} | ||
|
@@ -60,9 +68,9 @@ public void printHelp() { | |
System.out.println("Syntax:"); | ||
System.out.println( | ||
" rollback_to --warehouse <warehouse_path> --database <database_name> " | ||
+ "--table <table_name> --version <version_string>"); | ||
+ "--table <table_name> --version <version_string> --is_timestamp <is_timestamp>"); | ||
System.out.println( | ||
" <version_string> can be a long value representing a snapshot ID or a tag name."); | ||
" <version_string> can be a long value representing a snapshot ID or a tag name or a timestamp."); | ||
System.out.println(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,7 +28,7 @@ class RollbackProcedureTest extends PaimonSparkTestBase with StreamTest { | |
|
||
import testImplicits._ | ||
|
||
test("Paimon Procedure: rollback to snapshot and tag") { | ||
test("Paimon Procedure: rollback to snapshot and tag and timestamp") { | ||
failAfter(streamingTimeout) { | ||
withTempDir { | ||
checkpointDir => | ||
|
@@ -69,16 +69,31 @@ class RollbackProcedureTest extends PaimonSparkTestBase with StreamTest { | |
stream.processAllAvailable() | ||
checkAnswer(query(), Row(1, "a") :: Row(2, "b") :: Nil) | ||
|
||
val ts = System.currentTimeMillis | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. inline this |
||
|
||
// snapshot-3 | ||
inputData.addData((2, "b2")) | ||
stream.processAllAvailable() | ||
checkAnswer(query(), Row(1, "a") :: Row(2, "b2") :: Nil) | ||
|
||
// snapshot-4 | ||
inputData.addData((2, "b3")) | ||
stream.processAllAvailable() | ||
checkAnswer(query(), Row(1, "a") :: Row(2, "b3") :: Nil) | ||
|
||
assertThrows[RuntimeException] { | ||
spark.sql("CALL paimon.sys.rollback(table => 'test.T_exception', version => '2')") | ||
} | ||
// rollback to snapshot | ||
checkAnswer( | ||
spark.sql("CALL paimon.sys.rollback(table => 'test.T', version => '2')"), | ||
spark.sql("CALL paimon.sys.rollback(table => 'test.T', version => '3')"), | ||
Row(true) :: Nil) | ||
checkAnswer(query(), Row(1, "a") :: Row(2, "b2") :: Nil) | ||
|
||
// rollback with timestamp | ||
checkAnswer( | ||
spark.sql( | ||
s"CALL paimon.sys.rollback(table => 'test.T', version => '$ts', is_timestamp => true)"), | ||
Row(true) :: Nil) | ||
checkAnswer(query(), Row(1, "a") :: Row(2, "b") :: Nil) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use long timestamp uniformly in flink action && flink procedure && spark procedure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Firstly I also want to change it like you suggested, but consider older version spark user their procedure sql would not be compatible, so keep it can use in older version first. @wwj6591812