Skip to content

HBASE-28725 Use thirdparty protobuf for REST interface in HBase 2.x #6075

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stoty merged 1 commit into from
Sep 11, 2024
Merged

HBASE-28725 Use thirdparty protobuf for REST interface in HBase 2.x #6075

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 0 additions & 45 deletions hbase-protocol-shaded/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@
<description>Shaded protobuf protocol classes used by HBase internally.</description>
<properties>
<maven.javadoc.skip>true</maven.javadoc.skip>
<!--Version of protobuf that hbase uses internally (we shade our pb)
Must match what is out in hbase-thirdparty include.
-->
<internal.protobuf.version>4.27.3</internal.protobuf.version>
</properties>
<dependencies>
<!--BE CAREFUL! Any dependency added here needs to be
Expand Down Expand Up @@ -93,50 +89,9 @@
</execution>
</executions>
</plugin>
<!--Need this old plugin to replace in generated files instances
of com.google.protobuf so instead its o.a.h.h.com.google.protobuf.
Plugin is old and in google code archive. Here is usage done by
anohther: https://github.com/beiliubei/maven-replacer-plugin/wiki/Usage-Guide
The mess with the regex in the below is to prevent replacement every time
we run mvn install. There is probably a better way of avoiding the
double interpolation but this is it for now.
-->
<plugin>
<groupId>com.google.code.maven-replacer-plugin</groupId>
<artifactId>replacer</artifactId>
<version>1.5.3</version>
<configuration>
<basedir>${basedir}/target/generated-sources/</basedir>
<includes>
<include>**/*.java</include>
</includes>
<!-- Ignore errors when missing files, because it means this build
was run with -Dprotoc.skip and there is no -Dreplacer.skip -->
<ignoreErrors>true</ignoreErrors>
<replacements>
<replacement>
<token>([^\.])com.google.protobuf</token>
<value>$1org.apache.hbase.thirdparty.com.google.protobuf</value>
</replacement>
<replacement>
<token>(public)(\W+static)?(\W+final)?(\W+class)</token>
<value>@javax.annotation.Generated("proto") $1$2$3$4</value>
</replacement>
<!-- replacer doesn't support anchoring or negative lookbehind -->
<replacement>
<token>(@javax.annotation.Generated\("proto"\) ){2}</token>
<value>$1</value>
</replacement>
</replacements>
</configuration>
<executions>
<execution>
<goals>
<goal>replace</goal>
</goals>
<phase>process-sources</phase>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
Expand Down
14 changes: 12 additions & 2 deletions hbase-rest/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@
<artifactId>hbase-shaded-miscellaneous</artifactId>
</dependency>
<dependency>
<groupId>com.google.protobuf</groupId>
<artifactId>protobuf-java</artifactId>
<groupId>org.apache.hbase.thirdparty</groupId>
<artifactId>hbase-shaded-protobuf</artifactId>
</dependency>
<dependency>
<groupId>org.apache.httpcomponents</groupId>
Expand Down Expand Up @@ -358,6 +358,8 @@
</systemPropertyVariables>
</configuration>
</plugin>
<!-- The protobuf-maven-plugin and com.google.code.maven-replacer-plugin config
Copy link
Contributor

@Apache9 Apache9 Jul 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we move this to parent pom too? So we do not need to copy paste it everywhere...

Copy link
Contributor Author

@stoty stoty Jul 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will try to.

Copy link
Contributor Author

@stoty stoty Jul 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have centrilzed the replace plugin.
The protobuf compiler config cannot be centralized, as 2.x uses both protobuf 2.5 and thirdparty compilers.

is copied directly from hbase-shaded-protocol, and should be kept in sync. -->
<plugin>
<groupId>org.xolstice.maven.plugins</groupId>
<artifactId>protobuf-maven-plugin</artifactId>
Expand All @@ -368,9 +370,17 @@
<goal>compile</goal>
</goals>
<phase>generate-sources</phase>
<configuration>
<protocArtifact>com.google.protobuf:protoc:${internal.protobuf.version}:exe:${os.detected.classifier}</protocArtifact>
Copy link
Contributor

@Apache9 Apache9 Jul 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the only difference is this configuration? We could introduce a property for this so in hbase-protocol-shade and hbase-reset, we use the new protoc compiler while in hbase-protocol, we use the old one?

Copy link
Contributor Author

@stoty stoty Jul 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we could, but I think that the would hide the difference, and make it even harder to find which version we use where.

All the different protobuf libraries and compilers are hard enough figure out already, the current setup at least gives a clue to whoever tries to decypher it.

<checkStaleness>true</checkStaleness>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>com.google.code.maven-replacer-plugin</groupId>
<artifactId>replacer</artifactId>
</plugin>
<plugin>
<groupId>net.revelc.code</groupId>
<artifactId>warbucks-maven-plugin</artifactId>
Expand Down
7 changes: 4 additions & 3 deletions hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/ProtobufMessageHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,15 @@
*/
package org.apache.hadoop.hbase.rest;

import com.google.protobuf.CodedInputStream;
import com.google.protobuf.CodedOutputStream;
import com.google.protobuf.Message;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import org.apache.yetus.audience.InterfaceAudience;

import org.apache.hbase.thirdparty.com.google.protobuf.CodedInputStream;
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.
Expand Down
5 changes: 3 additions & 2 deletions hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/RestUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,16 @@
*/
package org.apache.hadoop.hbase.rest;

import com.google.protobuf.CodedInputStream;
import com.google.protobuf.Message;
import java.io.IOException;
import org.apache.hadoop.hbase.Cell;
import org.apache.hadoop.hbase.client.Result;
import org.apache.hadoop.hbase.rest.model.CellModel;
import org.apache.hadoop.hbase.rest.model.RowModel;
import org.apache.yetus.audience.InterfaceAudience;

import org.apache.hbase.thirdparty.com.google.protobuf.CodedInputStream;
import org.apache.hbase.thirdparty.com.google.protobuf.Message;

@InterfaceAudience.Private
public final class RestUtil {

Expand Down
13 changes: 7 additions & 6 deletions hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/CellModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@

import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.protobuf.CodedInputStream;
import com.google.protobuf.Message;
import java.io.IOException;
import java.io.Serializable;
import javax.xml.bind.annotation.XmlAccessType;
Expand All @@ -38,9 +36,12 @@
import org.apache.hadoop.hbase.rest.ProtobufMessageHandler;
import org.apache.hadoop.hbase.rest.RestUtil;
import org.apache.hadoop.hbase.rest.protobuf.generated.CellMessage.Cell;
import org.apache.hadoop.hbase.util.ByteStringer;
import org.apache.yetus.audience.InterfaceAudience;

import org.apache.hbase.thirdparty.com.google.protobuf.CodedInputStream;
import org.apache.hbase.thirdparty.com.google.protobuf.Message;
import org.apache.hbase.thirdparty.com.google.protobuf.UnsafeByteOperations;

/**
* Representation of a cell. A cell is a single value associated a column and optional qualifier,
* and either the timestamp when it was stored or the user- provided timestamp if one was explicitly
Expand Down Expand Up @@ -204,11 +205,11 @@ public int getValueLength() {
@Override
public Message messageFromObject() {
Cell.Builder builder = Cell.newBuilder();
builder.setColumn(ByteStringer.wrap(getColumn()));
builder.setColumn(UnsafeByteOperations.unsafeWrap(getColumn()));
if (valueLength == MAGIC_LENGTH) {
builder.setData(ByteStringer.wrap(getValue()));
builder.setData(UnsafeByteOperations.unsafeWrap(getValue()));
} else {
builder.setData(ByteStringer.wrap(value, valueOffset, valueLength));
builder.setData(UnsafeByteOperations.unsafeWrap(value, valueOffset, valueLength));
}
if (hasUserTimestamp()) {
builder.setTimestamp(getTimestamp());
Expand Down
21 changes: 11 additions & 10 deletions hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/CellSetModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

import static org.apache.hadoop.hbase.rest.model.CellModel.MAGIC_LENGTH;

import com.google.protobuf.CodedInputStream;
import com.google.protobuf.Message;
import java.io.IOException;
import java.io.Serializable;
import java.util.ArrayList;
Expand All @@ -34,9 +32,12 @@
import org.apache.hadoop.hbase.rest.RestUtil;
import org.apache.hadoop.hbase.rest.protobuf.generated.CellMessage.Cell;
import org.apache.hadoop.hbase.rest.protobuf.generated.CellSetMessage.CellSet;
import org.apache.hadoop.hbase.util.ByteStringer;
import org.apache.yetus.audience.InterfaceAudience;

import org.apache.hbase.thirdparty.com.google.protobuf.CodedInputStream;
import org.apache.hbase.thirdparty.com.google.protobuf.Message;
import org.apache.hbase.thirdparty.com.google.protobuf.UnsafeByteOperations;

/**
* Representation of a grouping of cells. May contain cells from more than one row. Encapsulates
* RowModel and CellModel models.
Expand Down Expand Up @@ -113,19 +114,19 @@ public Message messageFromObject() {
for (RowModel row : getRows()) {
CellSet.Row.Builder rowBuilder = CellSet.Row.newBuilder();
if (row.getKeyLength() == MAGIC_LENGTH) {
rowBuilder.setKey(ByteStringer.wrap(row.getKey()));
rowBuilder.setKey(UnsafeByteOperations.unsafeWrap(row.getKey()));
} else {
rowBuilder
.setKey(ByteStringer.wrap(row.getKeyArray(), row.getKeyOffset(), row.getKeyLength()));
rowBuilder.setKey(UnsafeByteOperations.unsafeWrap(row.getKeyArray(), row.getKeyOffset(),
row.getKeyLength()));
}
for (CellModel cell : row.getCells()) {
Cell.Builder cellBuilder = Cell.newBuilder();
cellBuilder.setColumn(ByteStringer.wrap(cell.getColumn()));
cellBuilder.setColumn(UnsafeByteOperations.unsafeWrap(cell.getColumn()));
if (cell.getValueLength() == MAGIC_LENGTH) {
cellBuilder.setData(ByteStringer.wrap(cell.getValue()));
cellBuilder.setData(UnsafeByteOperations.unsafeWrap(cell.getValue()));
} else {
cellBuilder.setData(
ByteStringer.wrap(cell.getValueArray(), cell.getValueOffset(), cell.getValueLength()));
cellBuilder.setData(UnsafeByteOperations.unsafeWrap(cell.getValueArray(),
cell.getValueOffset(), cell.getValueLength()));
}
if (cell.hasUserTimestamp()) {
cellBuilder.setTimestamp(cell.getTimestamp());
Expand Down
5 changes: 3 additions & 2 deletions hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/NamespacesInstanceModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
*/
package org.apache.hadoop.hbase.rest.model;

import com.google.protobuf.CodedInputStream;
import com.google.protobuf.Message;
import java.io.IOException;
import java.io.Serializable;
import java.util.HashMap;
Expand All @@ -35,6 +33,9 @@
import org.apache.hadoop.hbase.rest.protobuf.generated.NamespacePropertiesMessage.NamespaceProperties;
import org.apache.yetus.audience.InterfaceAudience;

import org.apache.hbase.thirdparty.com.google.protobuf.CodedInputStream;
import org.apache.hbase.thirdparty.com.google.protobuf.Message;

/**
* List a HBase namespace's key/value properties.
* <ul>
Expand Down
5 changes: 3 additions & 2 deletions hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/NamespacesModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
package org.apache.hadoop.hbase.rest.model;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.protobuf.CodedInputStream;
import com.google.protobuf.Message;
import java.io.IOException;
import java.io.Serializable;
import java.util.ArrayList;
Expand All @@ -35,6 +33,9 @@
import org.apache.hadoop.hbase.rest.protobuf.generated.NamespacesMessage.Namespaces;
import org.apache.yetus.audience.InterfaceAudience;

import org.apache.hbase.thirdparty.com.google.protobuf.CodedInputStream;
import org.apache.hbase.thirdparty.com.google.protobuf.Message;

/**
* A list of HBase namespaces.
* <ul>
Expand Down
5 changes: 3 additions & 2 deletions hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/RowModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
import static org.apache.hadoop.hbase.rest.model.CellModel.MAGIC_LENGTH;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.protobuf.CodedInputStream;
import com.google.protobuf.Message;
import java.io.IOException;
import java.io.Serializable;
import java.util.ArrayList;
Expand All @@ -38,6 +36,9 @@
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.yetus.audience.InterfaceAudience;

import org.apache.hbase.thirdparty.com.google.protobuf.CodedInputStream;
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.
Expand Down
14 changes: 7 additions & 7 deletions hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/ScannerModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@
package org.apache.hadoop.hbase.rest.model;

import com.fasterxml.jackson.annotation.JsonInclude;
import com.google.protobuf.ByteString;
import com.google.protobuf.CodedInputStream;
import com.google.protobuf.Message;
import java.io.IOException;
import java.io.Serializable;
import java.util.ArrayList;
Expand Down Expand Up @@ -74,12 +71,15 @@
import org.apache.hadoop.hbase.rest.RestUtil;
import org.apache.hadoop.hbase.rest.protobuf.generated.ScannerMessage.Scanner;
import org.apache.hadoop.hbase.security.visibility.Authorizations;
import org.apache.hadoop.hbase.util.ByteStringer;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.hbase.util.Pair;
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.CodedInputStream;
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;

/**
Expand Down Expand Up @@ -950,13 +950,13 @@ public void setFilter(String filter) {
public Message messageFromObject() {
Scanner.Builder builder = Scanner.newBuilder();
if (!Bytes.equals(startRow, HConstants.EMPTY_START_ROW)) {
builder.setStartRow(ByteStringer.wrap(startRow));
builder.setStartRow(UnsafeByteOperations.unsafeWrap(startRow));
}
if (!Bytes.equals(endRow, HConstants.EMPTY_START_ROW)) {
builder.setEndRow(ByteStringer.wrap(endRow));
builder.setEndRow(UnsafeByteOperations.unsafeWrap(endRow));
}
for (byte[] column : columns) {
builder.addColumns(ByteStringer.wrap(column));
builder.addColumns(UnsafeByteOperations.unsafeWrap(column));
}
if (startTime != 0) {
builder.setStartTime(startTime);
Expand Down
9 changes: 5 additions & 4 deletions hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/StorageClusterStatusModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
package org.apache.hadoop.hbase.rest.model;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.protobuf.CodedInputStream;
import com.google.protobuf.Message;
import java.io.IOException;
import java.io.Serializable;
import java.util.ArrayList;
Expand All @@ -31,10 +29,13 @@
import org.apache.hadoop.hbase.rest.ProtobufMessageHandler;
import org.apache.hadoop.hbase.rest.RestUtil;
import org.apache.hadoop.hbase.rest.protobuf.generated.StorageClusterStatusMessage.StorageClusterStatus;
import org.apache.hadoop.hbase.util.ByteStringer;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.yetus.audience.InterfaceAudience;

import org.apache.hbase.thirdparty.com.google.protobuf.CodedInputStream;
import org.apache.hbase.thirdparty.com.google.protobuf.Message;
import org.apache.hbase.thirdparty.com.google.protobuf.UnsafeByteOperations;

/**
* Representation of the status of a storage cluster:
* <p>
Expand Down Expand Up @@ -668,7 +669,7 @@ public Message messageFromObject() {
for (Node.Region region : node.regions) {
StorageClusterStatus.Region.Builder regionBuilder =
StorageClusterStatus.Region.newBuilder();
regionBuilder.setName(ByteStringer.wrap(region.name));
regionBuilder.setName(UnsafeByteOperations.unsafeWrap(region.name));
regionBuilder.setStores(region.stores);
regionBuilder.setStorefiles(region.storefiles);
regionBuilder.setStorefileSizeMB(region.storefileSizeMB);
Expand Down
11 changes: 6 additions & 5 deletions hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/TableInfoModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
*/
package org.apache.hadoop.hbase.rest.model;

import com.google.protobuf.CodedInputStream;
import com.google.protobuf.Message;
import java.io.IOException;
import java.io.Serializable;
import java.util.ArrayList;
Expand All @@ -29,9 +27,12 @@
import org.apache.hadoop.hbase.rest.ProtobufMessageHandler;
import org.apache.hadoop.hbase.rest.RestUtil;
import org.apache.hadoop.hbase.rest.protobuf.generated.TableInfoMessage.TableInfo;
import org.apache.hadoop.hbase.util.ByteStringer;
import org.apache.yetus.audience.InterfaceAudience;

import org.apache.hbase.thirdparty.com.google.protobuf.CodedInputStream;
import org.apache.hbase.thirdparty.com.google.protobuf.Message;
import org.apache.hbase.thirdparty.com.google.protobuf.UnsafeByteOperations;

/**
* Representation of a list of table regions.
*
Expand Down Expand Up @@ -130,8 +131,8 @@ public Message messageFromObject() {
TableInfo.Region.Builder regionBuilder = TableInfo.Region.newBuilder();
regionBuilder.setName(aRegion.getName());
regionBuilder.setId(aRegion.getId());
regionBuilder.setStartKey(ByteStringer.wrap(aRegion.getStartKey()));
regionBuilder.setEndKey(ByteStringer.wrap(aRegion.getEndKey()));
regionBuilder.setStartKey(UnsafeByteOperations.unsafeWrap(aRegion.getStartKey()));
regionBuilder.setEndKey(UnsafeByteOperations.unsafeWrap(aRegion.getEndKey()));
regionBuilder.setLocation(aRegion.getLocation());
builder.addRegions(regionBuilder);
}
Expand Down
Loading