You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by GitBox <gi...@apache.org> on 2021/05/28 06:55:46 UTC
[GitHub] [incubator-pegasus] levy5307 opened a new pull request #739: feat: implement value_schema_manager::get_value_schema
levy5307 opened a new pull request #739:
URL: https://github.com/apache/incubator-pegasus/pull/739
### What problem does this PR solve? <!--add issue link with summary if exists-->
proposal https://github.com/apache/incubator-pegasus/blob/master/rfcs/2020-10-09-data-version-v3.md
implement value_schema_manager::get_value_schema
### Checklist <!--REMOVE the items that are not applicable-->
##### Tests <!-- At least one of them must be included. -->
- Unit test
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] acelyc111 commented on a change in pull request #739: feat: implement value_schema_manager::get_value_schema
Posted by GitBox <gi...@apache.org>.
acelyc111 commented on a change in pull request #739:
URL: https://github.com/apache/incubator-pegasus/pull/739#discussion_r642333204
##########
File path: src/base/value_schema_manager.cpp
##########
@@ -42,8 +42,25 @@ void value_schema_manager::register_schema(std::unique_ptr<value_schema> schema)
value_schema *value_schema_manager::get_value_schema(uint32_t meta_cf_data_version,
dsn::string_view value) const
{
- /// TBD(zlw)
- return nullptr;
+ dsn::data_input input(value);
+ uint8_t first_byte = input.read_u8();
+ // first bit = 1 means the data version is >= VERSION_2
+ if (first_byte & 0x80) {
+ // In order to keep backward compatibility, we should return latest version if the data
+ // version in value is not found. In other words, it will works well in future version if it
Review comment:
```suggestion
// version in value is not found. In other words, it will work well in future version if it
```
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] hycdong merged pull request #739: feat: implement value_schema_manager::get_value_schema
Posted by GitBox <gi...@apache.org>.
hycdong merged pull request #739:
URL: https://github.com/apache/incubator-pegasus/pull/739
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] levy5307 commented on pull request #739: feat: implement value_schema_manager::get_value_schema
Posted by GitBox <gi...@apache.org>.
levy5307 commented on pull request #739:
URL: https://github.com/apache/incubator-pegasus/pull/739#issuecomment-851350250
> The old `value_schema` is a table level concept defined in `data_version` in meta column family. Now, `value_schema` version 2 updates it, `value_schema` becomes a value level concept, a table is permitted having different value schema. However, it will raise two problems.
> The first problem is the compatibility problem. New server version won't be downgraded because the old server can not figure out value_schema version 2 data. I think it is okay for new server can not downgrade, but our recent releases are almost all not-downgraded, should we merge this commit into master branch?
> The second problem is the `data_version` in meta cf. This value is actually useless for new server, but it can not be removed for version0 and version1, would you have any solution to remove this value in future?
I don't think so. Because value_schema version 2 will not updates it. It only reads it from meta cf.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] levy5307 edited a comment on pull request #739: feat: implement value_schema_manager::get_value_schema
Posted by GitBox <gi...@apache.org>.
levy5307 edited a comment on pull request #739:
URL: https://github.com/apache/incubator-pegasus/pull/739#issuecomment-851350250
> The old `value_schema` is a table level concept defined in `data_version` in meta column family. Now, `value_schema` version 2 updates it, `value_schema` becomes a value level concept, a table is permitted having different value schema. However, it will raise two problems.
> The first problem is the compatibility problem. New server version won't be downgraded because the old server can not figure out value_schema version 2 data. I think it is okay for new server can not downgrade, but our recent releases are almost all not-downgraded, should we merge this commit into master branch?
> The second problem is the `data_version` in meta cf. This value is actually useless for new server, but it can not be removed for version0 and version1, would you have any solution to remove this value in future?
I don't think so. Because value_schema version 2 will not updates data version in meta column family. It only reads from it.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] hycdong commented on pull request #739: feat: implement value_schema_manager::get_value_schema
Posted by GitBox <gi...@apache.org>.
hycdong commented on pull request #739:
URL: https://github.com/apache/incubator-pegasus/pull/739#issuecomment-851343468
The old `value_schema` is a table level concept defined in `data_version` in meta column family. Now, `value_schema` version 2 updates it, `value_schema` becomes a value level concept, a table is permitted having different value schema. However, it will raise two problems.
The first problem is the compatibility problem. New server version won't be downgraded because the old server can not figure out value_schema version 2 data. I think it is okay for new server can not downgrade, but our recent releases are almost all not-downgraded, should we merge this commit into master branch?
The second problem is the `data_version` in meta cf. This value is actually useless for new server, but it can not be removed for version0 and version1, would you have any solution to remove this value in future?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org