[
https://issues.apache.org/jira/browse/HBASE-3810?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13063597#comment-13063597
]
jirapos...@reviews.apache.org commented on HBASE-3810:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1051/#review1031
-----------------------------------------------------------
These changes look good and I'm probably being overly anal in my comments. But
since this configuration effectively forms part (a small part) of the "user
interface" for coprocessors, I think we should be careful what we codify here.
src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
<https://reviews.apache.org/r/1051/#comment2109>
This pattern is both looser and stricter in requirements for coprocessor
attribute keys. ie, this allows leading whitespace and case-insensitive
"coprocessor" in the attribute key, but it also codifies that the "coprocessor"
string must be immediately followed by a '$' delimiter (with no intervening
whitespace), which was not previously required. Is this intentional or should
we make that part optional as well -- "(\\$[0-9]*)?"
src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
<https://reviews.apache.org/r/1051/#comment2110>
I think this should start with "(.*)\\|" so a leading space (or other char)
is not required when not using a jar file.
src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
<https://reviews.apache.org/r/1051/#comment2111>
Same as above.
I also think we can combine this regex and the one above by making the last
bit for key=value configurations optional, but don't want to overly complicate
things. Would combining the two make the matching code more complex?
- Gary
On 2011-07-08 19:44:30, Mingjie Lai wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/1051/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-07-08 19:44:30)
bq.
bq.
bq. Review request for hbase.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Registering a Coprocessor at HTableDescriptor should be less strict
bq. - fix regex to account for whitespace around ":" separators
bq. - make path portion optional – we already skip the path handling if the
class can be loaded by the classloader
bq. - make priority optional and default to "USER"
bq.
bq.
bq. This addresses bug HBase-3810.
bq. https://issues.apache.org/jira/browse/HBase-3810
bq.
bq.
bq. Diffs
bq. -----
bq.
bq.
src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
c2b3558
bq. src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java
e0bde92
bq.
bq. Diff: https://reviews.apache.org/r/1051/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Tests passed locally.
bq.
bq.
bq. Thanks,
bq.
bq. Mingjie
bq.
bq.
> Registering a Coprocessor at HTableDescriptor should be less strict
> -------------------------------------------------------------------
>
> Key: HBASE-3810
> URL: https://issues.apache.org/jira/browse/HBASE-3810
> Project: HBase
> Issue Type: Improvement
> Components: coprocessors
> Affects Versions: 0.92.0
> Environment: all
> Reporter: Joerg Schad
> Assignee: Mingjie Lai
> Priority: Minor
> Fix For: 0.92.0
>
> Original Estimate: 2h
> Remaining Estimate: 2h
>
> Registering a Copressor in the following way will fail as the "Coprocessor$1"
> keyword is case sensitive (instead COPROCESSOR$1 works fine). Removing this
> restriction would improve usability.
> HTableDescriptor desc = new HTableDescriptor(tName);
> desc.setValue("Coprocessor$1",
> path.toString() + ":" + full_class_name +
> ":" + Coprocessor.Priority.USER);
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
