You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "nancyxu123 (via GitHub)" <gi...@apache.org> on 2023/02/03 20:37:42 UTC
[GitHub] [beam] nancyxu123 opened a new pull request, #25309: [BEAM-12164] Made sure state transitions were correct when updating partition state
nancyxu123 opened a new pull request, #25309:
URL: https://github.com/apache/beam/pull/25309
Ensure that we only update the Partition States in increasing order: CREATED -> SCHEDULED - > RUNNING -> FINISHED
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] github-actions[bot] commented on pull request #25309: [BEAM-12164] Made sure state transitions were correct when updating partition state
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25309:
URL: https://github.com/apache/beam/pull/25309#issuecomment-1445346086
Reminder, please take a look at this pr: @kennknowles @Abacn
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] github-actions[bot] commented on pull request #25309: [BEAM-12164] Made sure state transitions were correct when updating partition state
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25309:
URL: https://github.com/apache/beam/pull/25309#issuecomment-1435656461
Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment `assign to next reviewer`:
R: @kennknowles for label java.
R: @Abacn for label io.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] github-actions[bot] commented on pull request #25309: [BEAM-12164] Made sure state transitions were correct when updating partition state
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25309:
URL: https://github.com/apache/beam/pull/25309#issuecomment-1426732831
Reminder, please take a look at this pr: @apilloud @ahmedabu98
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] github-actions[bot] commented on pull request #25309: [BEAM-12164] Made sure state transitions were correct when updating partition state
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25309:
URL: https://github.com/apache/beam/pull/25309#issuecomment-1416428536
Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:
R: @apilloud for label java.
R: @ahmedabu98 for label io.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
The PR bot will only process comments in the main thread (not review comments).
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] thiagotnunes commented on a diff in pull request #25309: [BEAM-12164] Made sure state transitions were correct when updating partition state
Posted by "thiagotnunes (via GitHub)" <gi...@apache.org>.
thiagotnunes commented on code in PR #25309:
URL: https://github.com/apache/beam/pull/25309#discussion_r1096266289
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/dao/PartitionMetadataDao.java:
##########
@@ -390,11 +392,30 @@ public Void insert(PartitionMetadata row) {
* @param partitionTokens the partitions' unique identifiers
*/
public Void updateToScheduled(List<String> partitionTokens) {
- final List<Mutation> mutations =
- partitionTokens.stream()
- .map(token -> createUpdateMetadataStateMutationFrom(token, State.SCHEDULED))
- .collect(Collectors.toList());
- transaction.buffer(mutations);
+ HashMap<String, State> tokenToState = new HashMap<>();
+ Statement statement = createPartitionQueryStatement(partitionTokens);
+ try (ResultSet resultSet = transaction.executeQuery(statement)) {
+ while (resultSet.next()) {
+ if (resultSet.getString(COLUMN_PARTITION_TOKEN) == null
Review Comment:
Why would the partition token / state be `null`? I think they are actually `NOT NULL` columns
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/dao/PartitionMetadataDao.java:
##########
@@ -390,11 +392,30 @@ public Void insert(PartitionMetadata row) {
* @param partitionTokens the partitions' unique identifiers
*/
public Void updateToScheduled(List<String> partitionTokens) {
- final List<Mutation> mutations =
- partitionTokens.stream()
- .map(token -> createUpdateMetadataStateMutationFrom(token, State.SCHEDULED))
- .collect(Collectors.toList());
- transaction.buffer(mutations);
+ HashMap<String, State> tokenToState = new HashMap<>();
+ Statement statement = createPartitionQueryStatement(partitionTokens);
+ try (ResultSet resultSet = transaction.executeQuery(statement)) {
+ while (resultSet.next()) {
+ if (resultSet.getString(COLUMN_PARTITION_TOKEN) == null
+ || resultSet.getString(COLUMN_STATE) == null) {
+ continue;
+ }
+ tokenToState.put(
+ resultSet.getString(COLUMN_PARTITION_TOKEN),
+ State.valueOf(resultSet.getString(COLUMN_STATE)));
+ }
+ }
+
+ for (String partitionToken : partitionTokens) {
Review Comment:
Instead of programatically doing this, could we do a `SELECT * FROM metadata_table WHERE state = CREATED AND partition_token IN (@partitionTokens)`?
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/dao/PartitionMetadataDao.java:
##########
@@ -404,6 +425,18 @@ public Void updateToScheduled(List<String> partitionTokens) {
* @param partitionToken the partition unique identifier
*/
public Void updateToRunning(String partitionToken) {
+ Statement statement =
+ createPartitionQueryStatement(Collections.singletonList(partitionToken));
+ try (ResultSet resultSet = transaction.executeQuery(statement)) {
+ if (resultSet.next()) {
+ if (resultSet.getString(COLUMN_STATE) == null
Review Comment:
I don't think the state can be `NULL`
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] thiagotnunes commented on a diff in pull request #25309: [BEAM-12164] Made sure state transitions were correct when updating partition state
Posted by "thiagotnunes (via GitHub)" <gi...@apache.org>.
thiagotnunes commented on code in PR #25309:
URL: https://github.com/apache/beam/pull/25309#discussion_r1096356668
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/dao/PartitionMetadataDaoTest.java:
##########
@@ -165,6 +279,23 @@ public void testInTransactionContextUpdateToFinished() {
mutationValueMap.get(PartitionMetadataAdminDao.COLUMN_STATE).getString());
}
+ // @Test
Review Comment:
Remember to cleanup
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/it/ChangeStreamTestPipelineOptions.java:
##########
@@ -21,17 +21,17 @@
import org.apache.beam.sdk.options.Default;
import org.apache.beam.sdk.options.Description;
import org.apache.beam.sdk.options.StreamingOptions;
-import org.checkerframework.checker.nullness.qual.Nullable;
public interface ChangeStreamTestPipelineOptions extends IOTestPipelineOptions, StreamingOptions {
@Description("Project that hosts Spanner instance")
- @Nullable
+ // @Nullable
Review Comment:
Remember to cleanup
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] nancyxu123 closed pull request #25309: [BEAM-12164] Made sure state transitions were correct when updating partition state
Posted by "nancyxu123 (via GitHub)" <gi...@apache.org>.
nancyxu123 closed pull request #25309: [BEAM-12164] Made sure state transitions were correct when updating partition state
URL: https://github.com/apache/beam/pull/25309
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org