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
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 all commits
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

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

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
1 change: 1 addition & 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 @@ -166,4 +166,5 @@ public ProtobufMessageHandler getObjectFromMessage(byte[] message) throws IOExce
}
return this;
}

}
25 changes: 20 additions & 5 deletions hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestNamespacesInstanceResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import org.apache.hadoop.hbase.testclassification.MediumTests;
import org.apache.hadoop.hbase.testclassification.RestTests;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.http.Header;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.ClassRule;
Expand Down Expand Up @@ -329,6 +330,12 @@ public void testNamespaceCreateAndDeleteXMLAndJSON() throws IOException, JAXBExc
jsonString = jsonMapper.writeValueAsString(model2);
response = client.post(namespacePath2, Constants.MIMETYPE_JSON, Bytes.toBytes(jsonString));
assertEquals(201, response.getCode());
//check passing null content-type with a payload returns 415
Header[] nullHeaders = null;
response = client.post(namespacePath1, nullHeaders, toXML(model1));
assertEquals(415, response.getCode());
response = client.post(namespacePath1, nullHeaders, Bytes.toBytes(jsonString));
assertEquals(415, response.getCode());

// Check that created namespaces correctly.
nd1 = findNamespace(admin, NAMESPACE1);
Expand Down Expand Up @@ -379,16 +386,20 @@ public void testNamespaceCreateAndDeletePBAndNoBody() throws IOException, JAXBEx
model4 = testNamespacesInstanceModel.buildTestModel(NAMESPACE4, NAMESPACE4_PROPS);
testNamespacesInstanceModel.checkModel(model4, NAMESPACE4, NAMESPACE4_PROPS);

//Defines null headers for use in tests where no body content is provided, so that we set
// no content-type in the request
Header[] nullHeaders = null;

// Test cannot PUT (alter) non-existent namespace.
response = client.put(namespacePath3, Constants.MIMETYPE_BINARY, new byte[]{});
response = client.put(namespacePath3, nullHeaders, new byte[]{});
assertEquals(403, response.getCode());
response = client.put(namespacePath4, Constants.MIMETYPE_PROTOBUF,
model4.createProtobufOutput());
assertEquals(403, response.getCode());

// Test cannot create tables when in read only mode.
conf.set("hbase.rest.readonly", "true");
response = client.post(namespacePath3, Constants.MIMETYPE_BINARY, new byte[]{});
response = client.post(namespacePath3, nullHeaders, new byte[]{});
assertEquals(403, response.getCode());
response = client.put(namespacePath4, Constants.MIMETYPE_PROTOBUF,
model4.createProtobufOutput());
Expand All @@ -399,12 +410,16 @@ public void testNamespaceCreateAndDeletePBAndNoBody() throws IOException, JAXBEx
assertNull(nd4);
conf.set("hbase.rest.readonly", "false");

// Create namespace via no body and protobuf.
response = client.post(namespacePath3, Constants.MIMETYPE_BINARY, new byte[]{});
// Create namespace with no body and binary content type.
response = client.post(namespacePath3, nullHeaders, new byte[]{});
assertEquals(201, response.getCode());
// Create namespace with protobuf content-type.
response = client.post(namespacePath4, Constants.MIMETYPE_PROTOBUF,
model4.createProtobufOutput());
assertEquals(201, response.getCode());
//check setting unsupported content-type returns 415
response = client.post(namespacePath3, Constants.MIMETYPE_BINARY, new byte[]{});
assertEquals(415, response.getCode());

// Check that created namespaces correctly.
nd3 = findNamespace(admin, NAMESPACE3);
Expand All @@ -415,7 +430,7 @@ public void testNamespaceCreateAndDeletePBAndNoBody() throws IOException, JAXBEx
checkNamespaceProperties(nd4, NAMESPACE4_PROPS);

// Check cannot post tables that already exist.
response = client.post(namespacePath3, Constants.MIMETYPE_BINARY, new byte[]{});
response = client.post(namespacePath3, nullHeaders, new byte[]{});
assertEquals(403, response.getCode());
response = client.post(namespacePath4, Constants.MIMETYPE_PROTOBUF,
model4.createProtobufOutput());
Expand Down