-
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] Add TimeTravelUtils to resolve snapshot from scan params #4307
Conversation
Expansion :We can support more scan startup mode, like Add parameters : |
Thanks @LinMingQiang ,good reminder, had added timestamp config, but mode not add due to user not need add two config if they use snapshot or timestamp. |
Maybe you want |
Yes, it is a direct way, I had test for it, can query right snapshot and reset consumer. Thanks @JingsongLi |
import static org.junit.jupiter.api.Assertions.assertThrows; | ||
|
||
/** Tests for {@link TimeTravelUtilsTest}. */ | ||
public class TimeTravelUtilsTest extends ScannerTestBase { |
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.
@link TimeTravelUtil
CoreOptions.SCAN_TAG_NAME.key(), | ||
CoreOptions.SCAN_WATERMARK.key(), | ||
CoreOptions.SCAN_TIMESTAMP_MILLIS.key())); | ||
} |
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.
Use checkArgument
will be better.
scanKeys.add(CoreOptions.SCAN_TAG_NAME.key()); | ||
scanKeys.add(CoreOptions.SCAN_WATERMARK.key()); | ||
scanKeys.add(CoreOptions.SCAN_TIMESTAMP_MILLIS.key()); | ||
} |
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.
Use String[]{} is better.
if (scanHandleKey.size() > 1) { | ||
throw new IllegalArgumentException( | ||
String.format( | ||
"%s %s %s and %s can contains only one", |
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.
Only one of the following parameters may be set : [%s, %s, %s]
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.
Thanks @LinMingQiang had addressed
|
||
public static Snapshot resolveSnapshotFromOption( | ||
CoreOptions options, SnapshotManager snapshotManager) { | ||
List<String> scanHandleKey = new ArrayList<>(); |
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.
List scanHandleKey = new ArrayList<>(1);
/** Contains time travel functions. */ | ||
public class TimeTravelUtil { | ||
|
||
public static String[] scanKeys = { |
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.
private static final String[] SCAN_KEYS = {
CoreOptions.SCAN_TIMESTAMP_MILLIS.key() | ||
}; | ||
|
||
public static Snapshot resolveSnapshotFromOption( |
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.
resolveSnapshotFromOptions
} | ||
} | ||
|
||
if (scanHandleKey.size() == 0) { |
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.
Add a warn log?
return snapshot; | ||
} | ||
|
||
private static Snapshot handleSnapshotId(SnapshotManager snapshotManager, CoreOptions options) { |
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.
resolveSnapshotBySnapshotId
optMap.put("scan.timestamp-millis", ts + ""); | ||
options = CoreOptions.fromMap(optMap); | ||
snapshot = TimeTravelUtil.resolveSnapshotFromOption(options, snapshotManager); | ||
assertThat(snapshot.id() == 1); |
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.
assertTrue
optMap.put("scan.snapshot-id", "2"); | ||
CoreOptions options = CoreOptions.fromMap(optMap); | ||
Snapshot snapshot = TimeTravelUtil.resolveSnapshotFromOption(options, snapshotManager); | ||
assertThat(snapshot.id() == 2); |
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.
add assert snapshot.id() is not null
public class TimeTravelUtilsTest extends ScannerTestBase { | ||
|
||
@Test | ||
public void testScan() throws Exception { |
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.
testResolveSnapshotFromOptions
|
||
write.write(rowData(1, 10, 100L)); | ||
commit.commit(0, write.prepareCommit(true, 0)); | ||
long ts = System.currentTimeMillis(); |
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.
inline this
write.write(rowData(3, 50, 500L)); | ||
commit.commit(2, write.prepareCommit(true, 2)); | ||
|
||
HashMap<String, String> optMap = new HashMap<>(); |
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.
Map<String, String> optMap = new HashMap<>(4);
@wwj6591812 had addressd, Thanks. |
+1 |
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.
+1
Purpose
Add TimeTravelUtils to resolve snapshot from scan params
Linked issue: close #xxx
Tests
API and Format
Documentation