This is an automated email from the ASF dual-hosted git repository.

stoty pushed a commit to branch branch-3
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-3 by this push:
     new e23da545c8c HBASE-28613 Use streaming when marshalling protobuf REST 
output (#5943)
e23da545c8c is described below

commit e23da545c8c493b4906c60b3e7da873df4640753
Author: Istvan Toth <st...@apache.org>
AuthorDate: Tue May 28 10:21:24 2024 +0200

    HBASE-28613 Use streaming when marshalling protobuf REST output (#5943)
    
    Signed-off-by: Ankit Singhal <an...@apache.org>
    (cherry picked from commit d1d8b4d64591de6cee302d648be257087c0beb48)
---
 .../hadoop/hbase/rest/ProtobufMessageHandler.java  | 43 +++++++++++++++++++++-
 .../apache/hadoop/hbase/rest/model/CellModel.java  |  5 ++-
 .../hadoop/hbase/rest/model/CellSetModel.java      |  5 ++-
 .../hbase/rest/model/NamespacesInstanceModel.java  |  6 ++-
 .../hadoop/hbase/rest/model/NamespacesModel.java   |  6 ++-
 .../apache/hadoop/hbase/rest/model/RowModel.java   |  4 +-
 .../hadoop/hbase/rest/model/ScannerModel.java      |  5 ++-
 .../rest/model/StorageClusterStatusModel.java      |  5 ++-
 .../hadoop/hbase/rest/model/TableInfoModel.java    |  5 ++-
 .../hadoop/hbase/rest/model/TableListModel.java    |  6 ++-
 .../hadoop/hbase/rest/model/TableSchemaModel.java  |  6 ++-
 .../hadoop/hbase/rest/model/VersionModel.java      |  5 ++-
 .../producer/ProtobufMessageBodyProducer.java      |  2 +-
 13 files changed, 79 insertions(+), 24 deletions(-)

diff --git 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/ProtobufMessageHandler.java
 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/ProtobufMessageHandler.java
index 2e01ff24d47..962e5dfae86 100644
--- 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/ProtobufMessageHandler.java
+++ 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/ProtobufMessageHandler.java
@@ -18,21 +18,60 @@
 package org.apache.hadoop.hbase.rest;
 
 import java.io.IOException;
+import java.io.OutputStream;
 import org.apache.yetus.audience.InterfaceAudience;
 
+import org.apache.hbase.thirdparty.com.google.protobuf.CodedOutputStream;
+import org.apache.hbase.thirdparty.com.google.protobuf.Message;
+
 /**
  * Common interface for models capable of supporting protobuf marshalling and 
unmarshalling. Hooks
  * up to the ProtobufMessageBodyConsumer and ProtobufMessageBodyProducer 
adapters.
  */
 @InterfaceAudience.Private
 public interface ProtobufMessageHandler {
-  /** Returns the protobuf represention of the model */
-  byte[] createProtobufOutput();
+
+  // The Jetty 9.4 HttpOutput default commit size is 32K/4 = 8K. We use that 
size to avoid
+  // double buffering (and copying) in HttpOutput. If we ever increase the 
HttpOutput commit size,
+  // we need to adjust this accordingly. We should also revisit this when 
Jetty is upgraded.
+  int BUFFER_SIZE = 8 * 1024;
+
+  /** Writes the protobuf represention of the model to os */
+  default void writeProtobufOutput(OutputStream os) throws IOException {
+    // Creating an explicit CodedOutputStream for the following reasons :
+    // 1. This avoids the cost of pre-computing the message size
+    // 2. This lets us set the buffer size explicitly
+    CodedOutputStream cos = CodedOutputStream.newInstance(os, BUFFER_SIZE);
+    messageFromObject().writeTo(cos);
+    cos.flush();
+  }
+
+  /**
+   * Returns the protobuf represention of the model in a byte array Use
+   * {@link 
org.apache.hadoop.hbase.rest.ProtobufMessageHandler#writeProtobufOutput(OutputStream)}
+   * for better performance
+   * @return the protobuf encoded object in a byte array
+   */
+  default byte[] createProtobufOutput() {
+    return messageFromObject().toByteArray();
+  }
+
+  /**
+   * Convert to model to a protobuf Message object
+   * @return the protobuf Message object
+   */
+  Message messageFromObject();
 
   /**
    * Initialize the model from a protobuf representation.
    * @param message the raw bytes of the protobuf message
    * @return reference to self for convenience
    */
+  // TODO implement proper stream handling for unmarshalling.
+  // Using byte array here lets us use ProtobufUtil.mergeFrom in the 
implementations to
+  // avoid the CodedOutputStream size limitation, but is slow
+  // and memory intensive. We could use the ProtobufUtil.mergeFrom() variant 
that takes
+  // an inputStream and sets the size limit to maxInt.
+  // This would help both on the client side, and when processing large Puts 
on the server.
   ProtobufMessageHandler getObjectFromMessage(byte[] message) throws 
IOException;
 }
diff --git 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/CellModel.java 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/CellModel.java
index 4284727e438..3d8806b7dc0 100644
--- a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/CellModel.java
+++ b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/CellModel.java
@@ -36,6 +36,7 @@ import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.rest.ProtobufMessageHandler;
 import org.apache.yetus.audience.InterfaceAudience;
 
+import org.apache.hbase.thirdparty.com.google.protobuf.Message;
 import org.apache.hbase.thirdparty.com.google.protobuf.UnsafeByteOperations;
 
 import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
@@ -202,7 +203,7 @@ public class CellModel implements ProtobufMessageHandler, 
Serializable {
   }
 
   @Override
-  public byte[] createProtobufOutput() {
+  public Message messageFromObject() {
     Cell.Builder builder = Cell.newBuilder();
     builder.setColumn(UnsafeByteOperations.unsafeWrap(getColumn()));
     if (valueLength == MAGIC_LENGTH) {
@@ -213,7 +214,7 @@ public class CellModel implements ProtobufMessageHandler, 
Serializable {
     if (hasUserTimestamp()) {
       builder.setTimestamp(getTimestamp());
     }
-    return builder.build().toByteArray();
+    return builder.build();
   }
 
   @Override
diff --git 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/CellSetModel.java 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/CellSetModel.java
index 8908ec7e6c8..8486be2762f 100644
--- 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/CellSetModel.java
+++ 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/CellSetModel.java
@@ -31,6 +31,7 @@ import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.rest.ProtobufMessageHandler;
 import org.apache.yetus.audience.InterfaceAudience;
 
+import org.apache.hbase.thirdparty.com.google.protobuf.Message;
 import org.apache.hbase.thirdparty.com.google.protobuf.UnsafeByteOperations;
 
 import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
@@ -108,7 +109,7 @@ public class CellSetModel implements Serializable, 
ProtobufMessageHandler {
   }
 
   @Override
-  public byte[] createProtobufOutput() {
+  public Message messageFromObject() {
     CellSet.Builder builder = CellSet.newBuilder();
     for (RowModel row : getRows()) {
       CellSet.Row.Builder rowBuilder = CellSet.Row.newBuilder();
@@ -134,7 +135,7 @@ public class CellSetModel implements Serializable, 
ProtobufMessageHandler {
       }
       builder.addRows(rowBuilder);
     }
-    return builder.build().toByteArray();
+    return builder.build();
   }
 
   @Override
diff --git 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/NamespacesInstanceModel.java
 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/NamespacesInstanceModel.java
index 64b46f2956c..78f64720385 100644
--- 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/NamespacesInstanceModel.java
+++ 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/NamespacesInstanceModel.java
@@ -31,6 +31,8 @@ import org.apache.hadoop.hbase.client.Admin;
 import org.apache.hadoop.hbase.rest.ProtobufMessageHandler;
 import org.apache.yetus.audience.InterfaceAudience;
 
+import org.apache.hbase.thirdparty.com.google.protobuf.Message;
+
 import 
org.apache.hadoop.hbase.shaded.rest.protobuf.generated.NamespacePropertiesMessage.NamespaceProperties;
 
 /**
@@ -140,7 +142,7 @@ public class NamespacesInstanceModel implements 
Serializable, ProtobufMessageHan
   }
 
   @Override
-  public byte[] createProtobufOutput() {
+  public Message messageFromObject() {
     NamespaceProperties.Builder builder = NamespaceProperties.newBuilder();
     if (properties != null) {
       for (Map.Entry<String, String> entry : properties.entrySet()) {
@@ -151,7 +153,7 @@ public class NamespacesInstanceModel implements 
Serializable, ProtobufMessageHan
         builder.addProps(property);
       }
     }
-    return builder.build().toByteArray();
+    return builder.build();
   }
 
   @Override
diff --git 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/NamespacesModel.java
 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/NamespacesModel.java
index e866c7a935d..90e4f6560a5 100644
--- 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/NamespacesModel.java
+++ 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/NamespacesModel.java
@@ -31,6 +31,8 @@ import org.apache.hadoop.hbase.client.Admin;
 import org.apache.hadoop.hbase.rest.ProtobufMessageHandler;
 import org.apache.yetus.audience.InterfaceAudience;
 
+import org.apache.hbase.thirdparty.com.google.protobuf.Message;
+
 import 
org.apache.hadoop.hbase.shaded.rest.protobuf.generated.NamespacesMessage.Namespaces;
 
 /**
@@ -95,10 +97,10 @@ public class NamespacesModel implements Serializable, 
ProtobufMessageHandler {
   }
 
   @Override
-  public byte[] createProtobufOutput() {
+  public Message messageFromObject() {
     Namespaces.Builder builder = Namespaces.newBuilder();
     builder.addAllNamespace(namespaces);
-    return builder.build().toByteArray();
+    return builder.build();
   }
 
   @Override
diff --git 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/RowModel.java 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/RowModel.java
index 8b660ac362f..e200dfbc1f3 100644
--- a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/RowModel.java
+++ b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/RowModel.java
@@ -36,6 +36,8 @@ import org.apache.hadoop.hbase.rest.ProtobufMessageHandler;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.yetus.audience.InterfaceAudience;
 
+import org.apache.hbase.thirdparty.com.google.protobuf.Message;
+
 /**
  * Representation of a row. A row is a related set of cells, grouped by common 
row key. RowModels do
  * not appear in results by themselves. They are always encapsulated within 
CellSetModels.
@@ -179,7 +181,7 @@ public class RowModel implements ProtobufMessageHandler, 
Serializable {
   }
 
   @Override
-  public byte[] createProtobufOutput() {
+  public Message messageFromObject() {
     // there is no standalone row protobuf message
     throw new UnsupportedOperationException("no protobuf equivalent to 
RowModel");
   }
diff --git 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/ScannerModel.java 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/ScannerModel.java
index 3655a379804..4c241753e5e 100644
--- 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/ScannerModel.java
+++ 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/ScannerModel.java
@@ -71,6 +71,7 @@ import org.apache.yetus.audience.InterfaceAudience;
 
 import 
org.apache.hbase.thirdparty.com.fasterxml.jackson.jaxrs.json.JacksonJaxbJsonProvider;
 import org.apache.hbase.thirdparty.com.google.protobuf.ByteString;
+import org.apache.hbase.thirdparty.com.google.protobuf.Message;
 import org.apache.hbase.thirdparty.com.google.protobuf.UnsafeByteOperations;
 import org.apache.hbase.thirdparty.javax.ws.rs.core.MediaType;
 
@@ -809,7 +810,7 @@ public class ScannerModel implements 
ProtobufMessageHandler, Serializable {
   }
 
   @Override
-  public byte[] createProtobufOutput() {
+  public Message messageFromObject() {
     Scanner.Builder builder = Scanner.newBuilder();
     if (!Bytes.equals(startRow, HConstants.EMPTY_START_ROW)) {
       builder.setStartRow(UnsafeByteOperations.unsafeWrap(startRow));
@@ -842,7 +843,7 @@ public class ScannerModel implements 
ProtobufMessageHandler, Serializable {
         builder.addLabels(label);
     }
     builder.setCacheBlocks(cacheBlocks);
-    return builder.build().toByteArray();
+    return builder.build();
   }
 
   @Override
diff --git 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/StorageClusterStatusModel.java
 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/StorageClusterStatusModel.java
index e0102811142..c9370cad901 100644
--- 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/StorageClusterStatusModel.java
+++ 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/StorageClusterStatusModel.java
@@ -30,6 +30,7 @@ import org.apache.hadoop.hbase.rest.ProtobufMessageHandler;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.yetus.audience.InterfaceAudience;
 
+import org.apache.hbase.thirdparty.com.google.protobuf.Message;
 import org.apache.hbase.thirdparty.com.google.protobuf.UnsafeByteOperations;
 
 import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
@@ -672,7 +673,7 @@ public class StorageClusterStatusModel implements 
Serializable, ProtobufMessageH
   }
 
   @Override
-  public byte[] createProtobufOutput() {
+  public Message messageFromObject() {
     StorageClusterStatus.Builder builder = StorageClusterStatus.newBuilder();
     builder.setRegions(regions);
     builder.setRequests(requests);
@@ -708,7 +709,7 @@ public class StorageClusterStatusModel implements 
Serializable, ProtobufMessageH
     for (String node : deadNodes) {
       builder.addDeadNodes(node);
     }
-    return builder.build().toByteArray();
+    return builder.build();
   }
 
   @Override
diff --git 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/TableInfoModel.java
 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/TableInfoModel.java
index 74d0732ec91..43b131fcb70 100644
--- 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/TableInfoModel.java
+++ 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/TableInfoModel.java
@@ -27,6 +27,7 @@ import javax.xml.bind.annotation.XmlRootElement;
 import org.apache.hadoop.hbase.rest.ProtobufMessageHandler;
 import org.apache.yetus.audience.InterfaceAudience;
 
+import org.apache.hbase.thirdparty.com.google.protobuf.Message;
 import org.apache.hbase.thirdparty.com.google.protobuf.UnsafeByteOperations;
 
 import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
@@ -123,7 +124,7 @@ public class TableInfoModel implements Serializable, 
ProtobufMessageHandler {
   }
 
   @Override
-  public byte[] createProtobufOutput() {
+  public Message messageFromObject() {
     TableInfo.Builder builder = TableInfo.newBuilder();
     builder.setName(name);
     for (TableRegionModel aRegion : regions) {
@@ -135,7 +136,7 @@ public class TableInfoModel implements Serializable, 
ProtobufMessageHandler {
       regionBuilder.setLocation(aRegion.getLocation());
       builder.addRegions(regionBuilder);
     }
-    return builder.build().toByteArray();
+    return builder.build();
   }
 
   @Override
diff --git 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/TableListModel.java
 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/TableListModel.java
index 76854acdf6a..63b2e809279 100644
--- 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/TableListModel.java
+++ 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/TableListModel.java
@@ -26,6 +26,8 @@ import javax.xml.bind.annotation.XmlRootElement;
 import org.apache.hadoop.hbase.rest.ProtobufMessageHandler;
 import org.apache.yetus.audience.InterfaceAudience;
 
+import org.apache.hbase.thirdparty.com.google.protobuf.Message;
+
 import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
 import 
org.apache.hadoop.hbase.shaded.rest.protobuf.generated.TableListMessage.TableList;
 
@@ -90,12 +92,12 @@ public class TableListModel implements Serializable, 
ProtobufMessageHandler {
   }
 
   @Override
-  public byte[] createProtobufOutput() {
+  public Message messageFromObject() {
     TableList.Builder builder = TableList.newBuilder();
     for (TableModel aTable : tables) {
       builder.addName(aTable.getName());
     }
-    return builder.build().toByteArray();
+    return builder.build();
   }
 
   @Override
diff --git 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/TableSchemaModel.java
 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/TableSchemaModel.java
index 06abe355859..f2a8c4c7060 100644
--- 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/TableSchemaModel.java
+++ 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/TableSchemaModel.java
@@ -42,6 +42,8 @@ import org.apache.hadoop.hbase.rest.ProtobufMessageHandler;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.yetus.audience.InterfaceAudience;
 
+import org.apache.hbase.thirdparty.com.google.protobuf.Message;
+
 import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
 import 
org.apache.hadoop.hbase.shaded.rest.protobuf.generated.ColumnSchemaMessage.ColumnSchema;
 import 
org.apache.hadoop.hbase.shaded.rest.protobuf.generated.TableSchemaMessage.TableSchema;
@@ -248,7 +250,7 @@ public class TableSchemaModel implements Serializable, 
ProtobufMessageHandler {
   }
 
   @Override
-  public byte[] createProtobufOutput() {
+  public Message messageFromObject() {
     TableSchema.Builder builder = TableSchema.newBuilder();
     builder.setName(name);
     for (Map.Entry<QName, Object> e : attrs.entrySet()) {
@@ -281,7 +283,7 @@ public class TableSchemaModel implements Serializable, 
ProtobufMessageHandler {
     if (attrs.containsKey(READONLY)) {
       
builder.setReadOnly(Boolean.parseBoolean(attrs.get(READONLY).toString()));
     }
-    return builder.build().toByteArray();
+    return builder.build();
   }
 
   @Override
diff --git 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/VersionModel.java 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/VersionModel.java
index e5d79af5e55..65eca57ac5a 100644
--- 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/VersionModel.java
+++ 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/VersionModel.java
@@ -26,6 +26,7 @@ import org.apache.hadoop.hbase.rest.ProtobufMessageHandler;
 import org.apache.hadoop.hbase.rest.RESTServlet;
 import org.apache.yetus.audience.InterfaceAudience;
 
+import org.apache.hbase.thirdparty.com.google.protobuf.Message;
 import 
org.apache.hbase.thirdparty.org.glassfish.jersey.servlet.ServletContainer;
 
 import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
@@ -162,14 +163,14 @@ public class VersionModel implements Serializable, 
ProtobufMessageHandler {
   }
 
   @Override
-  public byte[] createProtobufOutput() {
+  public Message messageFromObject() {
     Version.Builder builder = Version.newBuilder();
     builder.setRestVersion(restVersion);
     builder.setJvmVersion(jvmVersion);
     builder.setOsVersion(osVersion);
     builder.setServerVersion(serverVersion);
     builder.setJerseyVersion(jerseyVersion);
-    return builder.build().toByteArray();
+    return builder.build();
   }
 
   @Override
diff --git 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/provider/producer/ProtobufMessageBodyProducer.java
 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/provider/producer/ProtobufMessageBodyProducer.java
index 1d95e6f343e..4a7806e652b 100644
--- 
a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/provider/producer/ProtobufMessageBodyProducer.java
+++ 
b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/provider/producer/ProtobufMessageBodyProducer.java
@@ -59,6 +59,6 @@ public class ProtobufMessageBodyProducer implements 
MessageBodyWriter<ProtobufMe
   public void writeTo(ProtobufMessageHandler m, Class<?> type, Type 
genericType,
     Annotation[] annotations, MediaType mediaType, MultivaluedMap<String, 
Object> httpHeaders,
     OutputStream entityStream) throws IOException, WebApplicationException {
-    entityStream.write(m.createProtobufOutput());
+    m.writeProtobufOutput(entityStream);
   }
 }

Reply via email to