[
https://issues.apache.org/jira/browse/HBASE-15198?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15139036#comment-15139036
]
stack commented on HBASE-15198:
-------------------------------
Patch looks good. Change some wording on commit.
So, we are talking about an attribute of a particular client connection. The
attribute setting can change per client connection.
What happens to this attribute if we ask to do cell blocks but the server
responds that it does not support them. IIRC, this is possible.
This javadoc is a little 'worrying':
306 /**
307 * @return true when this connection uses a {@link
org.apache.hadoop.hbase.codec.Codec} and so
308 * supports cell blocks rather than Protobuffed Cells.
309 */
If I read it and didn't know anything about how our rpc worked, I'd be
wondering if cell blocks are a good thing or not and perhaps I should be pb'ing
Cells instead (that sounds 'better').
Can we point to some place else in javadoc that explains implications of using
cell blocks? Otherwise just drop the 'rather than Protobuffed Cells'.
Above can be done on commit.
Should this be a define for true in below?
if (!(connection instanceof ClusterConnection)) return true;
Should the name of this method be hasCellBlockSupport.... i.e. javabean it --
so just reading name of the method we know its result is boolean.
supportsCellBlock
The javadoc below is repeated...
88 * @return true when this client uses a {@link
org.apache.hadoop.hbase.codec.Codec} and so
89 * supports cell blocks rather than Protobuffed Cells.
.... so change here too. Give impression that CellBlock'ing is good... the
default. When would we not cellblock?
Removal of this is intentional?
1191 if(cell.getTagsLength() > 0) {
1192 valueBuilder.setTags(ByteStringer.wrap(cell.getTagsArray(),
cell.getTagsOffset(),
1193 cell.getTagsLength()));
1194 }
+1 on patch. Fix above on commit.
> RPC client not using Codec and CellBlock for puts by default
> ------------------------------------------------------------
>
> Key: HBASE-15198
> URL: https://issues.apache.org/jira/browse/HBASE-15198
> Project: HBase
> Issue Type: Bug
> Affects Versions: 0.98.0
> Reporter: Anoop Sam John
> Assignee: Anoop Sam John
> Priority: Critical
> Fix For: 2.0.0, 1.3.0, 1.2.1, 1.1.4, 1.0.4, 0.98.18
>
> Attachments: HBASE-15198.patch, HBASE-15198_V2.patch,
> HBASE-15198_V3.patch, HBASE-15198_V4.patch, HBASE-15198_V5.patch
>
>
> For puts we use MultiServerCallable. Here to decide whether to use cellBlock
> we have
> {code}
> private boolean isCellBlock() {
> // This is not exact -- the configuration could have changed on us after
> connection was set up
> // but it will do for now.
> HConnection connection = getConnection();
> if (connection == null) return true; // Default is to do cellblocks.
> Configuration configuration = connection.getConfiguration();
> if (configuration == null) return true;
> String codec = configuration.get(HConstants.RPC_CODEC_CONF_KEY, "");
> return codec != null && codec.length() > 0;
> }
> {code}
> By default in hbase-default.xml, we dont have any Codec being specified.
> Where as in AbstractRpcClient we have
> {code}
> Codec getCodec() {
> // For NO CODEC, "hbase.client.rpc.codec" must be configured with empty
> string AND
> // "hbase.client.default.rpc.codec" also -- because default is to do cell
> block encoding.
> String className = conf.get(HConstants.RPC_CODEC_CONF_KEY,
> getDefaultCodec(this.conf));
> if (className == null || className.length() == 0) return null;
> try {
> return (Codec)Class.forName(className).newInstance();
> } catch (Exception e) {
> throw new RuntimeException("Failed getting codec " + className, e);
> }
> }
> .....
> public static String getDefaultCodec(final Configuration c) {
> // If "hbase.client.default.rpc.codec" is empty string -- you can't set
> it to null because
> // Configuration will complain -- then no default codec (and we'll pb
> everything). Else
> // default is KeyValueCodec
> return c.get(DEFAULT_CODEC_CLASS, KeyValueCodec.class.getCanonicalName());
> }
> {code}
> Our aim is to by def use Codec and it is KeyValueCodec.
> The codec finding in MultiServerCallable to be same way as in
> AbstractRpcClient and then only we will be doing cellblock stuff.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
