Skip to content

HBASE-22846 Internal Error 500 when Using HBASE REST API to Create Na… #524

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
wchevreuil merged 4 commits into from
Sep 17, 2019
Merged

HBASE-22846 Internal Error 500 when Using HBASE REST API to Create Na… #524

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c638d8e
HBASE-22846 Internal Error 500 when Using HBASE REST API to Create Na…
wchevreuil Aug 22, 2019
10ef12d
removed test main
wchevreuil Aug 30, 2019
d0c00ca
temp changes
wchevreuil Sep 2, 2019
384ea47
fixing UT to avoid usage of unsupported content-type (application/oct…
wchevreuil Sep 2, 2019
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
69 changes: 16 additions & 53 deletions hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/NamespacesInstanceResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,35 +135,9 @@ public Response get(final @Context ServletContext context,
@Consumes({MIMETYPE_XML, MIMETYPE_JSON, MIMETYPE_PROTOBUF,
MIMETYPE_PROTOBUF_IETF})
public Response put(final NamespacesInstanceModel model, final @Context UriInfo uriInfo) {
if (LOG.isTraceEnabled()) {
LOG.trace("PUT " + uriInfo.getAbsolutePath());
}
servlet.getMetrics().incrementRequests(1);
return processUpdate(model, true, uriInfo);
}

/**
* Build a response for PUT alter namespace with no properties specified.
* @param message value not used.
* @param headers value not used.
* @return response code.
*/
@PUT
public Response putNoBody(final byte[] message,
Copy link
Contributor

@saintstack saintstack Sep 14, 2019

Choose a reason for hiding this comment

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

I don't know how this works. When you remove this, the put gets called?

Copy link
Contributor Author

@wchevreuil wchevreuil Sep 16, 2019

Choose a reason for hiding this comment

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

For some reason, on branch-2 based versions, these putNoBody and postNoBody methods only get properly called if client sets content-type request header to "application/octet-stream". We don't explicit define any requirement for "binary" request content-type, on the REST API doc in the ref guide, so it seems clients had been working well without setting content-type, in hbase 1. I suspect the behaviour change might be due jersey version changes between branch-1 and branch-2.
This fix simply removed the additional method, so request is guaranteed to always go to same place and then payload validations are applied there.

final @Context UriInfo uriInfo, final @Context HttpHeaders headers) {
if (LOG.isTraceEnabled()) {
LOG.trace("PUT " + uriInfo.getAbsolutePath());
}
servlet.getMetrics().incrementRequests(1);
try{
NamespacesInstanceModel model = new NamespacesInstanceModel(namespace);
return processUpdate(model, true, uriInfo);
}catch(IOException ioe){
servlet.getMetrics().incrementFailedPutRequests(1);
throw new RuntimeException("Cannot retrieve info for '" + namespace + "'.");
}
}

/**
* Build a response for POST create namespace with properties specified.
* @param model properties used for create.
Expand All @@ -175,39 +149,26 @@ public Response putNoBody(final byte[] message,
MIMETYPE_PROTOBUF_IETF})
public Response post(final NamespacesInstanceModel model,
final @Context UriInfo uriInfo) {

if (LOG.isTraceEnabled()) {
LOG.trace("POST " + uriInfo.getAbsolutePath());
}
servlet.getMetrics().incrementRequests(1);
return processUpdate(model, false, uriInfo);
}

/**
* Build a response for POST create namespace with no properties specified.
* @param message value not used.
* @param headers value not used.
* @return response code.
*/
@POST
public Response postNoBody(final byte[] message,
final @Context UriInfo uriInfo, final @Context HttpHeaders headers) {

// Check that POST or PUT is valid and then update namespace.
private Response processUpdate(NamespacesInstanceModel model, final boolean updateExisting,
final UriInfo uriInfo) {
if (LOG.isTraceEnabled()) {
LOG.trace("POST " + uriInfo.getAbsolutePath());
LOG.trace((updateExisting ? "PUT " : "POST ") + uriInfo.getAbsolutePath());
}
servlet.getMetrics().incrementRequests(1);
try{
NamespacesInstanceModel model = new NamespacesInstanceModel(namespace);
return processUpdate(model, false, uriInfo);
}catch(IOException ioe){
servlet.getMetrics().incrementFailedPutRequests(1);
throw new RuntimeException("Cannot retrieve info for '" + namespace + "'.");
if (model == null) {
try {
model = new NamespacesInstanceModel(namespace);
} catch(IOException ioe) {
servlet.getMetrics().incrementFailedPutRequests(1);
throw new RuntimeException("Cannot retrieve info for '" + namespace + "'.");
}
}
}
servlet.getMetrics().incrementRequests(1);

// Check that POST or PUT is valid and then update namespace.
private Response processUpdate(final NamespacesInstanceModel model, final boolean updateExisting,
final UriInfo uriInfo) {
if (servlet.isReadOnly()) {
servlet.getMetrics().incrementFailedPutRequests(1);
return Response.status(Response.Status.FORBIDDEN).type(MIMETYPE_TEXT)
Expand Down Expand Up @@ -265,7 +226,9 @@ private Response createOrUpdate(final NamespacesInstanceModel model, final UriIn
}

servlet.getMetrics().incrementSucessfulPutRequests(1);
return Response.created(uriInfo.getAbsolutePath()).build();

return updateExisting ? Response.ok(uriInfo.getAbsolutePath()).build() :
Response.created(uriInfo.getAbsolutePath()).build();
}

private boolean doesNamespaceExist(Admin admin, String namespaceName) throws IOException{
Expand Down
14 changes: 14 additions & 0 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 @@ -30,6 +30,7 @@
import javax.xml.bind.annotation.XmlRootElement;
import javax.xml.bind.annotation.XmlTransient;

import com.google.gson.Gson;
import org.apache.hadoop.hbase.NamespaceDescriptor;
import org.apache.yetus.audience.InterfaceAudience;
import org.apache.hadoop.hbase.client.Admin;
Expand Down Expand Up @@ -166,4 +167,17 @@ public ProtobufMessageHandler getObjectFromMessage(byte[] message) throws IOExce
}
return this;
}

public static void main(String[] args) {
Copy link
Contributor

@busbey busbey Aug 25, 2019

Choose a reason for hiding this comment

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

Can we do this as a unit test instead of a main method someone has to run?

Copy link
Contributor Author

@wchevreuil wchevreuil Aug 27, 2019

Choose a reason for hiding this comment

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

This should actually be removed, just had temporarily added this in order to have a proper json string generated, to perform manual tests using an http client.

NamespacesInstanceModel model = new NamespacesInstanceModel();
model.addProperty("test", "test value");
model.addProperty("test1", "test value 1");
model.namespaceName = "name";

Gson gson = new Gson();
String json = gson.toJson(model);
System.out.println(json);


}
}