[
https://issues.apache.org/jira/browse/HBASE-1400?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12715251#action_12715251
]
Brian Beggs commented on HBASE-1400:
------------------------------------
If I had the opportunity to go back and write the HBase REST interface now,
after knowing a lot more about Hbase and having used Jersey on another project,
this is how I would want to write the interface.
What I like:
- Having Jersey integrated with the project takes away so much manual work that
was being done before with the Response messages and input parsing. That alone
will make this implementation so much less error prone that what is currently
in the interface. I spent a lot of time chasing down I/O defects in the
current implementation which left me less time to focus on functionality.
- Generally everything about this implementation is much cleaner than prior
implementations.
What I would like to see:
- I do think it would be beneficial to have JSON support since the current
interface supports this. I would be willing to contribute this portion.
Thoughts:
- I think that having JSON I/O is still beneficial to the interface. While the
addition of protobufs makes for an attractive option to JSON, there may still
be a need for a JSON interface for something such as JavaScript (Only Firefox
is officially supported for JS protobufs).
- As with the prior implementation there is some redundant code centering
around parsing/serializing of I/O. For example in the TableResources class
there are 3 methods to serialize output (text, xml and protobufs). I had tried
to centralize alot of this serialization code into it's own class using the
visitor pattern for the last iteration to keep the implementation classes a
little cleaner, reduce code redundancy and to make it a bit easier to extend
the interface to add new I/O types. While the old interface had redundancy in
the fact that there was serialization or parsing for each data type, I feel
that the I/O is wrapped a bit too tightly in this current implementation and
more closely resembles the REST implementation before I made the I/O more
modular.
I think that there is a bit of redundancy in the implementation as it stands
currently, and I also think that it can be factored out relatively easily...
(see below)
- I think it would be desirable to add some model classes to the interface that
could be used for I/O parsing/Serialization. These model classes could use the
JAX-B XML and JSON parsing/serialization that is already available with Jersey.
This would make the code more modular and easier to extend. Then to parse the
protobuf (or if the JAX-B implementations are not fast enough) adding a
provider to handle the I/O is relatively easy. Take a look at this article:
http://www.javarants.com/2008/12/27/using-jax-rs-with-protocol-buffers-for-high-performance-rest-apis/
Google says that protobufs generated classes should not be treated as first
class objects, and while I don't feel that this implementation necessary treats
them as such. I think there would be benefit from wrapping the protobufs into
the model classes discussed above and accessing them this way.
Taking all of the above into consideration expanding the REST interface becomes
much easier.
For example:
Lets say we want to add an additional piece of information into the list of
tables that gets returned when you call the root of the webapp @Path("/")
- Current implementation:
{code:title=TableResource.java (partial)|borderStyle=solid}
@GET
@Produces(MIMETYPE_TEXT)
public Response getAsText(@Context UriInfo uriInfo) {
if (LOG.isDebugEnabled()) {
LOG.debug("GET " + uriInfo.getAbsolutePath() + " as " + MIMETYPE_TEXT);
}
try {
RESTServer.getInstance().serviceRequests++;
} catch (IOException e) {
throw new WebApplicationException(e,
Response.Status.INTERNAL_SERVER_ERROR);
}
try {
StringWriter writer = new StringWriter();
for (HTableDescriptor htd: getTableList()) {
if (htd.isMetaRegion()) {
continue;
}
writer.append(htd.getNameAsString());
writer.append('\n');
}
ResponseBuilder response = Response.ok(writer.toString());
response.cacheControl(cacheControl);
return response.build();
} catch (IOException e) {
throw new WebApplicationException(e,
Response.Status.SERVICE_UNAVAILABLE);
}
}
@GET
@Produces(MIMETYPE_XML)
public Response getAsXML(@Context UriInfo uriInfo) {
if (LOG.isDebugEnabled()) {
LOG.debug("GET " + uriInfo.getAbsolutePath() + " as " + MIMETYPE_XML);
}
try {
RESTServer.getInstance().serviceRequests++;
} catch (IOException e) {
throw new WebApplicationException(e,
Response.Status.INTERNAL_SERVER_ERROR);
}
try {
StringWriter writer = new StringWriter();
XMLOutputter result = new XMLOutputter(writer, "US-ASCII");
result.startTag("TableList");
for (HTableDescriptor htd: getTableList()) {
if (htd.isMetaRegion()) {
continue;
}
result.startTag("table");
result.attribute("name", htd.getNameAsString());
result.endTag();
}
result.endTag();
result.endDocument();
ResponseBuilder response = Response.ok(writer.toString());
response.cacheControl(cacheControl);
return response.build();
} catch (UnsupportedEncodingException e) {
throw new WebApplicationException(e,
Response.Status.INTERNAL_SERVER_ERROR);
} catch (IOException e) {
throw new WebApplicationException(e,
Response.Status.SERVICE_UNAVAILABLE);
}
}
@GET
@Produces(MIMETYPE_PROTOBUF)
public Response getAsProtobuf(@Context UriInfo uriInfo) {
if (LOG.isDebugEnabled()) {
LOG.debug("GET " + uriInfo.getAbsolutePath() + " as " +
MIMETYPE_PROTOBUF);
}
try {
RESTServer.getInstance().serviceRequests++;
} catch (IOException e) {
throw new WebApplicationException(e,
Response.Status.INTERNAL_SERVER_ERROR);
}
try {
TableList.Builder list = TableList.newBuilder();
for (HTableDescriptor htd: getTableList()) {
if (htd.isMetaRegion()) {
continue;
}
list.addName(htd.getNameAsString());
}
ResponseBuilder response = Response.ok(list.build().toByteArray());
response.cacheControl(cacheControl);
return response.build();
} catch (IOException e) {
throw new WebApplicationException(e,
Response.Status.SERVICE_UNAVAILABLE);
}
}
{code}
To modify the code change the following:
-* Modify each method that serializes the output in the TableResource class
(currently 3, but +1 if we add JSON).
-* recreate the protobuf and recompile. (then make sure that this doesn't
effect any of the other classes that use this file)
So this is at least 4 methods you have to touch + n methods that may be
effected by the protobuf change.
- Modified implementation:
{code:title=TableResource.java (partial)|borderStyle=solid}
@GET
@Produces(MIMETYPE_TEXT, MIMETYPE_XML, MIMETYPE_PROTOBUF)
public TableModel get(@Context UriInfo uriInfo) {
List<TableModel> tables = new ArrayList<TableModel>();
if (LOG.isDebugEnabled()) {
LOG.debug("GET " + uriInfo.getAbsolutePath() + " as " + MIMETYPE_TEXT);
}
try {
RESTServer.getInstance().serviceRequests++;
} catch (IOException e) {
throw new WebApplicationException(e,
Response.Status.INTERNAL_SERVER_ERROR);
}
try {
for (HTableDescriptor htd: getTableList()) {
if (htd.isMetaRegion()) {
continue;
}
tables.add(new TableModel(htd.getName()));
}
return models;
} catch (IOException e) {
throw new WebApplicationException(e,
Response.Status.SERVICE_UNAVAILABLE);
}
}
{code}
{code:title=TableModel.java (partial)|borderStyle=solid}
@XmlRootElement(name="table")
@XmlType(name = "", propOrder = {"name"})
public class TableModel implements Serializable {
private static final long serialVersionUID = 2871762412753722057L;
private String name;
/**
* @param name
*/
public TableModel(String name) {
super();
this.name = name;
}
public TableModel() {}
/**
* @return the name
*/
public String getName() {
return name;
}
/**
* @param name the name to set
*/
public void setName(String name) {
this.name = name;
}
}
{code}
To modify the code change the following:
-* Add the field into the model object with the annotation
-* modify the 1 method that does the work to get the list of tables
-* make sure that the modified protobuf class doesn't effect the 1 class that
wraps it.
This is 3 methods that need to be modified.
I think that a modifying the implementation using these ideas makes for a
cleaner easier to maintain interface.
I would like to contribute back to this and will see if I can get some time on
my schedule for it.
@Stack {quote}Andrew: You've been busy. This looks great. How much does it
differ from current REST (I've not done a side-by-side but they seem
close){quote}
No the interfaces are not the same, but I think that if you guys want to have a
functional, easier to maintain REST implementation I think that the semantics
of this system are much better than what is available currently.
Overall I think that it's good work and would ultimately be a great
contribution to the project.
And that's my $3.50. Let me know if you guys would like any clarification.
> Improve REST interface semantics and efficiency
> -----------------------------------------------
>
> Key: HBASE-1400
> URL: https://issues.apache.org/jira/browse/HBASE-1400
> Project: Hadoop HBase
> Issue Type: Improvement
> Components: rest
> Reporter: Andrew Purtell
> Priority: Minor
> Attachments: stargate-0.0.1.zip, stargate-testing.pdf, stargate.pdf
>
>
> Improve the semantics of the REST interface: more metadata operations, bulk
> updates, protobufs (if Accept equals "application/x-protobuf" for GET or
> Content-Type equals the same for PUT or POST) instead of multipart/related
> (which is not supported now anyway) etc. for general efficiency and support
> for queries or scanners that return multiple KeyValues.
> I am working on a proposal.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
