You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/05/20 06:32:06 UTC
[GitHub] [flink] twalthr opened a new pull request #12263: [FLINK-16998][core] Support backwards compatibility for upgraded RowSerializer
twalthr opened a new pull request #12263:
URL: https://github.com/apache/flink/pull/12263
## What is the purpose of the change
Allows schema migration of the old serialization format for `RowSerializer`. The PR also updates the row serializer tests to the new `TypeSerializerUpgradeTestBase`.
Since `Row` is `PublicEvolving` we can drop the old serialization format soon.
## Brief change log
See commit messages.
## Verifying this change
This change added tests and can be verified as follows: `RowSerializerUpgradeTest`
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: no
- The serializers: yes
- The runtime per-record code paths (performance sensitive): no
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn/Mesos, ZooKeeper: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? no
- If yes, how is the feature documented? JavaDocs
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12263: [FLINK-16998][core] Support backwards compatibility for upgraded RowSerializer
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12263:
URL: https://github.com/apache/flink/pull/12263#issuecomment-631274882
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5e0f9df0a404a5d88b8762238ec37b903b9f0e4b",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1910",
"triggerID" : "5e0f9df0a404a5d88b8762238ec37b903b9f0e4b",
"triggerType" : "PUSH"
}, {
"hash" : "0e1d9cde275d0717fb9b32f6d1a3aed600c33166",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "0e1d9cde275d0717fb9b32f6d1a3aed600c33166",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5e0f9df0a404a5d88b8762238ec37b903b9f0e4b Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1910)
* 0e1d9cde275d0717fb9b32f6d1a3aed600c33166 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] twalthr closed pull request #12263: [FLINK-16998][core] Support backwards compatibility for upgraded RowSerializer
Posted by GitBox <gi...@apache.org>.
twalthr closed pull request #12263:
URL: https://github.com/apache/flink/pull/12263
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] twalthr commented on pull request #12263: [FLINK-16998][core] Support backwards compatibility for upgraded RowSerializer
Posted by GitBox <gi...@apache.org>.
twalthr commented on pull request #12263:
URL: https://github.com/apache/flink/pull/12263#issuecomment-631385082
Thanks for the feedback @tzulitai. After some offline discussion, the tests were partially incorrect. I hope the PR is in a better shape now.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12263: [FLINK-16998][core] Support backwards compatibility for upgraded RowSerializer
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12263:
URL: https://github.com/apache/flink/pull/12263#issuecomment-631274882
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5e0f9df0a404a5d88b8762238ec37b903b9f0e4b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1910",
"triggerID" : "5e0f9df0a404a5d88b8762238ec37b903b9f0e4b",
"triggerType" : "PUSH"
}, {
"hash" : "0e1d9cde275d0717fb9b32f6d1a3aed600c33166",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1933",
"triggerID" : "0e1d9cde275d0717fb9b32f6d1a3aed600c33166",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 0e1d9cde275d0717fb9b32f6d1a3aed600c33166 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1933)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12263: [FLINK-16998][core] Support backwards compatibility for upgraded RowSerializer
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12263:
URL: https://github.com/apache/flink/pull/12263#issuecomment-631274882
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5e0f9df0a404a5d88b8762238ec37b903b9f0e4b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1910",
"triggerID" : "5e0f9df0a404a5d88b8762238ec37b903b9f0e4b",
"triggerType" : "PUSH"
}, {
"hash" : "0e1d9cde275d0717fb9b32f6d1a3aed600c33166",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1933",
"triggerID" : "0e1d9cde275d0717fb9b32f6d1a3aed600c33166",
"triggerType" : "PUSH"
}, {
"hash" : "320f0a551c635e98c4aff4af6d853d3cf2681fee",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1944",
"triggerID" : "320f0a551c635e98c4aff4af6d853d3cf2681fee",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 0e1d9cde275d0717fb9b32f6d1a3aed600c33166 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1933)
* 320f0a551c635e98c4aff4af6d853d3cf2681fee Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1944)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12263: [FLINK-16998][core] Support backwards compatibility for upgraded RowSerializer
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12263:
URL: https://github.com/apache/flink/pull/12263#issuecomment-631274882
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5e0f9df0a404a5d88b8762238ec37b903b9f0e4b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1910",
"triggerID" : "5e0f9df0a404a5d88b8762238ec37b903b9f0e4b",
"triggerType" : "PUSH"
}, {
"hash" : "0e1d9cde275d0717fb9b32f6d1a3aed600c33166",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1933",
"triggerID" : "0e1d9cde275d0717fb9b32f6d1a3aed600c33166",
"triggerType" : "PUSH"
}, {
"hash" : "320f0a551c635e98c4aff4af6d853d3cf2681fee",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1944",
"triggerID" : "320f0a551c635e98c4aff4af6d853d3cf2681fee",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 320f0a551c635e98c4aff4af6d853d3cf2681fee Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1944)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] tzulitai commented on a change in pull request #12263: [FLINK-16998][core] Support backwards compatibility for upgraded RowSerializer
Posted by GitBox <gi...@apache.org>.
tzulitai commented on a change in pull request #12263:
URL: https://github.com/apache/flink/pull/12263#discussion_r427970574
##########
File path: flink-core/src/main/java/org/apache/flink/api/java/typeutils/runtime/RowSerializer.java
##########
@@ -367,21 +367,21 @@ public int getVersion() {
/**
* A {@link TypeSerializerSnapshot} for RowSerializer.
*/
- // TODO not fully functional yet due to FLINK-17520
public static final class RowSerializerSnapshot extends CompositeTypeSerializerSnapshot<Row, RowSerializer> {
private static final int VERSION = 3;
- private static final int VERSION_WITHOUT_ROW_KIND = 2;
+ private static final int LAST_VERSION_WITHOUT_ROW_KIND = 2;
- private boolean legacyModeEnabled = false;
+ private int readVersion = VERSION;
public RowSerializerSnapshot() {
super(RowSerializer.class);
}
RowSerializerSnapshot(RowSerializer serializerInstance) {
super(serializerInstance);
+ this.readVersion = serializerInstance.legacyModeEnabled ? LAST_VERSION_WITHOUT_ROW_KIND : VERSION;
Review comment:
I don't think this line is needed, unless I'm missing something in the tests.
The read version should only ever be changed if this snapshot was created by restoring from a snapshot.
In this case, this constructor is only ever used to create a new snapshot when checkpointing occurs - the read version should be the default value (`VALUE`).
##########
File path: flink-core/src/main/java/org/apache/flink/api/java/typeutils/runtime/RowSerializer.java
##########
@@ -60,7 +60,7 @@
public static final int ROW_KIND_OFFSET = 2;
- private static final long serialVersionUID = 2L;
+ private static final long serialVersionUID = 1L; // legacy, don't touch
Review comment:
nit: maybe add a comment that this can only be touched after support for 1.9 savepoints is ditched.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12263: [FLINK-16998][core] Support backwards compatibility for upgraded RowSerializer
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12263:
URL: https://github.com/apache/flink/pull/12263#issuecomment-631274882
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5e0f9df0a404a5d88b8762238ec37b903b9f0e4b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1910",
"triggerID" : "5e0f9df0a404a5d88b8762238ec37b903b9f0e4b",
"triggerType" : "PUSH"
}, {
"hash" : "0e1d9cde275d0717fb9b32f6d1a3aed600c33166",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1933",
"triggerID" : "0e1d9cde275d0717fb9b32f6d1a3aed600c33166",
"triggerType" : "PUSH"
}, {
"hash" : "320f0a551c635e98c4aff4af6d853d3cf2681fee",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "320f0a551c635e98c4aff4af6d853d3cf2681fee",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 0e1d9cde275d0717fb9b32f6d1a3aed600c33166 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1933)
* 320f0a551c635e98c4aff4af6d853d3cf2681fee UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12263: [FLINK-16998][core] Support backwards compatibility for upgraded RowSerializer
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12263:
URL: https://github.com/apache/flink/pull/12263#issuecomment-631267389
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.
## Automated Checks
Last check on commit 320f0a551c635e98c4aff4af6d853d3cf2681fee (Fri Oct 16 10:52:35 UTC 2020)
**Warnings:**
* **1 pom.xml files were touched**: Check for build and licensing issues.
* No documentation files were touched! Remember to keep the Flink docs up to date!
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] twalthr commented on pull request #12263: [FLINK-16998][core] Support backwards compatibility for upgraded RowSerializer
Posted by GitBox <gi...@apache.org>.
twalthr commented on pull request #12263:
URL: https://github.com/apache/flink/pull/12263#issuecomment-631460366
Thanks @tzulitai. I addressed the remaining comments and will merge this once the build gives green light.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #12263: [FLINK-16998][core] Support backwards compatibility for upgraded RowSerializer
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #12263:
URL: https://github.com/apache/flink/pull/12263#issuecomment-631274882
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5e0f9df0a404a5d88b8762238ec37b903b9f0e4b",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "5e0f9df0a404a5d88b8762238ec37b903b9f0e4b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5e0f9df0a404a5d88b8762238ec37b903b9f0e4b UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] twalthr commented on pull request #12263: [FLINK-16998][core] Support backwards compatibility for upgraded RowSerializer
Posted by GitBox <gi...@apache.org>.
twalthr commented on pull request #12263:
URL: https://github.com/apache/flink/pull/12263#issuecomment-631688716
Merged.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #12263: [FLINK-16998][core] Support backwards compatibility for upgraded RowSerializer
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #12263:
URL: https://github.com/apache/flink/pull/12263#issuecomment-631267389
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.
## Automated Checks
Last check on commit 5e0f9df0a404a5d88b8762238ec37b903b9f0e4b (Wed May 20 06:34:22 UTC 2020)
**Warnings:**
* **1 pom.xml files were touched**: Check for build and licensing issues.
* No documentation files were touched! Remember to keep the Flink docs up to date!
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12263: [FLINK-16998][core] Support backwards compatibility for upgraded RowSerializer
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12263:
URL: https://github.com/apache/flink/pull/12263#issuecomment-631274882
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5e0f9df0a404a5d88b8762238ec37b903b9f0e4b",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1910",
"triggerID" : "5e0f9df0a404a5d88b8762238ec37b903b9f0e4b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5e0f9df0a404a5d88b8762238ec37b903b9f0e4b Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1910)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] tzulitai commented on a change in pull request #12263: [FLINK-16998][core] Support backwards compatibility for upgraded RowSerializer
Posted by GitBox <gi...@apache.org>.
tzulitai commented on a change in pull request #12263:
URL: https://github.com/apache/flink/pull/12263#discussion_r427782378
##########
File path: flink-core/src/main/java/org/apache/flink/api/java/typeutils/runtime/RowSerializer.java
##########
@@ -367,36 +367,42 @@ public int getVersion() {
/**
* A {@link TypeSerializerSnapshot} for RowSerializer.
*/
- // TODO not fully functional yet due to FLINK-17520
public static final class RowSerializerSnapshot extends CompositeTypeSerializerSnapshot<Row, RowSerializer> {
private static final int VERSION = 3;
- private static final int VERSION_WITHOUT_ROW_KIND = 2;
+ private static final int LAST_VERSION_WITHOUT_ROW_KIND = 2;
- private boolean legacyModeEnabled = false;
+ private int version = VERSION;
public RowSerializerSnapshot() {
super(RowSerializer.class);
}
RowSerializerSnapshot(RowSerializer serializerInstance) {
super(serializerInstance);
+ this.version = translateVersion(serializerInstance);
}
@Override
protected int getCurrentOuterSnapshotVersion() {
- return VERSION;
+ return version;
Review comment:
This method is only ever relevant for when writing snapshots and not used on restore.
Therefore, this should always be the latest version, and not the read older version.
```suggestion
return VERSION;
```
##########
File path: flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/RowSerializerUpgradeTest.java
##########
@@ -76,14 +84,16 @@ public RowSerializerUpgradeTest(TestSpecification<Row, Row> testSpecification) {
public static final class RowSerializerSetup implements TypeSerializerUpgradeTestBase.PreUpgradeSetup<Row> {
@Override
public TypeSerializer<Row> createPriorSerializer() {
- return stringLongRowSupplier();
+ return createRowSerializer(true);
Review comment:
To really clarify this, I think we should make the `RowSerializer` constructor that allows passing in the `legacyModeEnabled` flag private, to be only usable by the `RowSerializerSnapshot#createOuterSerializer`. This concern should not be leaked into tests.
The bottom line is, the concern of creating an old serializer with previous formats should only be visible to the snapshots.
##########
File path: flink-core/src/main/java/org/apache/flink/api/java/typeutils/runtime/RowSerializer.java
##########
@@ -367,36 +367,42 @@ public int getVersion() {
/**
* A {@link TypeSerializerSnapshot} for RowSerializer.
*/
- // TODO not fully functional yet due to FLINK-17520
public static final class RowSerializerSnapshot extends CompositeTypeSerializerSnapshot<Row, RowSerializer> {
private static final int VERSION = 3;
- private static final int VERSION_WITHOUT_ROW_KIND = 2;
+ private static final int LAST_VERSION_WITHOUT_ROW_KIND = 2;
- private boolean legacyModeEnabled = false;
+ private int version = VERSION;
Review comment:
Maybe rename this to `readVersion`, to better convey its difference with the static `VERSION`.
##########
File path: flink-core/src/main/java/org/apache/flink/api/java/typeutils/runtime/RowSerializer.java
##########
@@ -367,36 +367,42 @@ public int getVersion() {
/**
* A {@link TypeSerializerSnapshot} for RowSerializer.
*/
- // TODO not fully functional yet due to FLINK-17520
public static final class RowSerializerSnapshot extends CompositeTypeSerializerSnapshot<Row, RowSerializer> {
private static final int VERSION = 3;
- private static final int VERSION_WITHOUT_ROW_KIND = 2;
+ private static final int LAST_VERSION_WITHOUT_ROW_KIND = 2;
- private boolean legacyModeEnabled = false;
+ private int version = VERSION;
public RowSerializerSnapshot() {
super(RowSerializer.class);
}
RowSerializerSnapshot(RowSerializer serializerInstance) {
super(serializerInstance);
+ this.version = translateVersion(serializerInstance);
}
@Override
protected int getCurrentOuterSnapshotVersion() {
- return VERSION;
+ return version;
}
@Override
protected void readOuterSnapshot(
int readOuterSnapshotVersion,
DataInputView in,
ClassLoader userCodeClassLoader) {
- if (readOuterSnapshotVersion == VERSION_WITHOUT_ROW_KIND) {
- legacyModeEnabled = true;
+ version = readOuterSnapshotVersion;
+ }
+
+ @Override
+ protected OuterSchemaCompatibility resolveOuterSchemaCompatibility(RowSerializer newSerializer) {
+ if (version == translateVersion(newSerializer)) {
Review comment:
If I understood this correctly, this check doesn't need to be performed against the new serializer.
Since starting 1.11 the created new serializer is always writing in the new format (e.g. `legacyModeEnabled` is always `false`), combined with the fact that the version for the `RowSerializerSnapshot` was up-ticked in 1.11,
whether or not migration is needed is actually completely encoded / "piggy-backed" in the version of the `RowSerializerSnapshot`:
i.e. this should be able to be simplified to:
```suggestion
if (readVersion <= LAST_VERSION_WITHOUT_ROW_KIND)) {
return OuterSchemaCompatibility.COMPATIBLE_AFTER_MIGRATION;
}
return OuterSchemaCompatibility.COMPATIBLE_AS_IS;
```
##########
File path: flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/RowSerializerUpgradeTest.java
##########
@@ -76,14 +84,16 @@ public RowSerializerUpgradeTest(TestSpecification<Row, Row> testSpecification) {
public static final class RowSerializerSetup implements TypeSerializerUpgradeTestBase.PreUpgradeSetup<Row> {
@Override
public TypeSerializer<Row> createPriorSerializer() {
- return stringLongRowSupplier();
+ return createRowSerializer(true);
Review comment:
I think there's a mis-understanding in the use of this `createPriorSerializer` method.
This method is only ever used when writing the test snapshot files, under the target branch.
e.g.
- if we checkout to the `release-1.10` branch and generate a snapshot, this should create a serializer that writes in the old format.
- if we checkout to the `release-1.11` branch and generate a snapshot, this should create a serializer that writes in the new format.
With the current changes in the PR, the generated snapshot files will always be written with the old format, regardless of which branch you're on.
```suggestion
// in older branches, this writes in old format;
// in newer branches >= 1.11, this writes in new format
return new RowSerializer(fieldSerializers);
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12263: [FLINK-16998][core] Support backwards compatibility for upgraded RowSerializer
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12263:
URL: https://github.com/apache/flink/pull/12263#issuecomment-631274882
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5e0f9df0a404a5d88b8762238ec37b903b9f0e4b",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1910",
"triggerID" : "5e0f9df0a404a5d88b8762238ec37b903b9f0e4b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5e0f9df0a404a5d88b8762238ec37b903b9f0e4b Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1910)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12263: [FLINK-16998][core] Support backwards compatibility for upgraded RowSerializer
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12263:
URL: https://github.com/apache/flink/pull/12263#issuecomment-631274882
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5e0f9df0a404a5d88b8762238ec37b903b9f0e4b",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1910",
"triggerID" : "5e0f9df0a404a5d88b8762238ec37b903b9f0e4b",
"triggerType" : "PUSH"
}, {
"hash" : "0e1d9cde275d0717fb9b32f6d1a3aed600c33166",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1933",
"triggerID" : "0e1d9cde275d0717fb9b32f6d1a3aed600c33166",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5e0f9df0a404a5d88b8762238ec37b903b9f0e4b Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1910)
* 0e1d9cde275d0717fb9b32f6d1a3aed600c33166 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=1933)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org