You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2020/06/20 06:00:52 UTC
[GitHub] [helix] pkuwm opened a new pull request #1108: Add compression and encode support for get propertyStore endpoint
pkuwm opened a new pull request #1108:
URL: https://github.com/apache/helix/pull/1108
### Issues
- [ ] My PR addresses the following Helix issues and references them in the PR description:
Fixed #1105
### Description
- [ ] Here are some details about my PR, including screenshots of any UI changes:
Get propertyStore endpoint creates a string response for zlib compressed bytes which causes client not able to decompress the data.
This PR adds compression and encode support to decompress/encode the compressed bytes properly.
For example:
- "/propertyStore/path?compression=zlib"
- "/propertyStore/path?encode=base64"
### Tests
- [ ] The following tests are written for this issue:
- testGetPropertyStoreWithZLibCompression
- testGetPropertyStoreWithGzipCompression
- testGetPropertyStoreWithEncode
- [ ] The following is the result of the "mvn test" command on the appropriate module:
(Copy & paste the result of "mvn test")
### Commits
- [ ] My commits all reference appropriate Apache Helix GitHub issues in their subject lines. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Documentation (Optional)
- [ ] In case of new functionality, my PR adds documentation in the following wiki page:
(Link the GitHub wiki you added)
### Code Quality
- [ ] My diff has been formatted using helix-style.xml
(helix-style-intellij.xml if IntelliJ IDE is used)
----------------------------------------------------------------
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: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] jiajunwang commented on pull request #1108: Add compression and encode support for get propertyStore endpoint
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on pull request #1108:
URL: https://github.com/apache/helix/pull/1108#issuecomment-647887465
Base64 is fine, but the other support for different compression algorithms won't be scalable.
I think we should stop at base64 support in the Helix rest level. And then add a Java API based on the rest API to provide a way that application client can pass their original customized ZkSerializer and get the correct data back.
----------------------------------------------------------------
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: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] kaisun2000 commented on a change in pull request #1108: Add compression and encode support for get propertyStore endpoint
Posted by GitBox <gi...@apache.org>.
kaisun2000 commented on a change in pull request #1108:
URL: https://github.com/apache/helix/pull/1108#discussion_r445208758
##########
File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/PropertyStoreAccessor.java
##########
@@ -158,4 +187,59 @@ public Response deletePropertyByPath(@PathParam("clusterId") String clusterId,
}
return OK();
}
+
+ private byte[] readPropertyStoreBytes(String recordPath) {
+ BaseDataAccessor<byte[]> propertyStoreDataAccessor = getByteArrayDataAccessor();
+
+ // If znode does not exist, bytes is null. But content of an existing znode could be null.
+ // We use stat ctime to check if znode exists or not. ctime == 0L means znode does not exist.
+ Stat stat = new Stat();
+ byte[] bytes = propertyStoreDataAccessor.get(recordPath, stat, AccessOption.PERSISTENT);
+ if (stat.getCtime() == 0L) {
+ throw new WebApplicationException(Response.status(Response.Status.NOT_FOUND)
+ .entity(String.format("The property store path %s doesn't exist", recordPath)).build());
+ }
+
+ return bytes;
+ }
+
+ private Response decompressBytes(byte[] bytes, String recordPath, String compression) {
+ byte[] decompressedBytes;
+ try {
+ switch (compression) {
+ case ZLIB_COMPRESSION:
+ case DEFLATER_COMPRESSION:
+ decompressedBytes = ZLibCompressionUtil.decompress(bytes);
+ break;
+ case GZIP_COMPRESSION:
+ decompressedBytes = GZipCompressionUtil.uncompress(new ByteArrayInputStream(bytes));
+ break;
+ default:
+ LOG.info("{} compression method is not supported.", compression);
Review comment:
LOG.ERROR()?
----------------------------------------------------------------
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: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] pkuwm closed pull request #1108: Add compression and encode support for get propertyStore endpoint
Posted by GitBox <gi...@apache.org>.
pkuwm closed pull request #1108:
URL: https://github.com/apache/helix/pull/1108
----------------------------------------------------------------
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: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] pkuwm closed pull request #1108: Add compression and encode support for get propertyStore endpoint
Posted by GitBox <gi...@apache.org>.
pkuwm closed pull request #1108:
URL: https://github.com/apache/helix/pull/1108
----------------------------------------------------------------
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: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] pkuwm commented on pull request #1108: Add compression and encode support for get propertyStore endpoint
Posted by GitBox <gi...@apache.org>.
pkuwm commented on pull request #1108:
URL: https://github.com/apache/helix/pull/1108#issuecomment-660834272
> Base64 is fine, but the other support for different compression algorithms won't be scalable.
> I think we should stop at base64 support in the Helix rest level. And then add a Java API based on the rest API to provide a way that application client can pass their original customized ZkSerializer and get the correct data back.
Had an offline sync with @jiajunwang . Will clean up the compression algorithm and keep only base64 support.
----------------------------------------------------------------
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: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org