[jira] [Commented] (HBASE-18029) Backport HBASE-15296 to branch-1

2017-05-12 Thread Appy (JIRA) 

[ 
https://issues.apache.org/jira/browse/HBASE-18029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16009036#comment-16009036
 ] 

Appy commented on HBASE-18029:
--

If we don't want to break coproc compat either, we can use StoreFile.Reader 
(instead of StoreFileReader) in cp interfaces too. That way, we might not have 
to typecast either.

Not sure about binary compat, but it'll be source-code compat at the least.

> Backport HBASE-15296 to branch-1
> 
>
> Key: HBASE-18029
> URL: https://issues.apache.org/jira/browse/HBASE-18029
> Project: HBase
>  Issue Type: Improvement
>  Components: regionserver
>Reporter: Duo Zhang
> Attachments: HBASE-18029.branch-1.001.patch
>
>




--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (HBASE-18029) Backport HBASE-15296 to branch-1

2017-05-12 Thread Andrew Purtell (JIRA) 

[ 
https://issues.apache.org/jira/browse/HBASE-18029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16008896#comment-16008896
 ] 

Andrew Purtell commented on HBASE-18029:


How can we do this refactor without breaking coprocessors on branch-1 ? 

> Backport HBASE-15296 to branch-1
> 
>
> Key: HBASE-18029
> URL: https://issues.apache.org/jira/browse/HBASE-18029
> Project: HBase
>  Issue Type: Improvement
>  Components: regionserver
>Reporter: Duo Zhang
> Attachments: HBASE-18029.branch-1.001.patch
>
>




--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (HBASE-18029) Backport HBASE-15296 to branch-1

2017-05-12 Thread Appy (JIRA) 

[ 
https://issues.apache.org/jira/browse/HBASE-18029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16008848#comment-16008848
 ] 

Appy commented on HBASE-18029:
--

I haven't thought this completely through, but throwing it out for discussion:
- Refactor out current nested classes to StoreFileReader / StoreFileWriter.
- Create dummy classes. For eg. {{class Reader extends StoreFileReader}}
- Return StoreFile.Reader from fns in StoreFile (keeps compat)
- return StoreFileReader from CP functions
- caveat being doing explicit casting in non-public parts of code. In this 
case, {{private Reader open(...)}}. But if we do se, we should explicitly 
comment dummy classes saying what they are and why.
(uploading a quick patch)

+1 for interfaces, but that's orthogonal, right? (unless am missing something)
[~Apache9]. Took me time to get back since i was on leave for majority of week.

> Backport HBASE-15296 to branch-1
> 
>
> Key: HBASE-18029
> URL: https://issues.apache.org/jira/browse/HBASE-18029
> Project: HBase
>  Issue Type: Improvement
>  Components: regionserver
>Reporter: Duo Zhang
>




--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (HBASE-18029) Backport HBASE-15296 to branch-1

2017-05-11 Thread Duo Zhang (JIRA) 

[ 
https://issues.apache.org/jira/browse/HBASE-18029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16007496#comment-16007496
 ] 

Duo Zhang commented on HBASE-18029:
---

HBASE-15296 is an incompatible change and [~appy] wants to provide a compatible 
backport to branch-1 which would make the upgrade more smoothly I guess? And 
for me, it can make the backporting of HBASE-17914 and HBASE-17917 much easier. 
But as I found that StoreFile is also marked as IA.LimitedPrivate, I do not 
think we need to backport those issues then...

Let me open a new issue to abstract StoreFile.

Thanks.

> Backport HBASE-15296 to branch-1
> 
>
> Key: HBASE-18029
> URL: https://issues.apache.org/jira/browse/HBASE-18029
> Project: HBase
>  Issue Type: Improvement
>  Components: regionserver
>Reporter: Duo Zhang
>




--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (HBASE-18029) Backport HBASE-15296 to branch-1

2017-05-11 Thread Andrew Purtell (JIRA) 

[ 
https://issues.apache.org/jira/browse/HBASE-18029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16006863#comment-16006863
 ] 

Andrew Purtell commented on HBASE-18029:


What is the rationale for backporting HBASE-15296 to branch-1? I don't see it 
here. 

> Backport HBASE-15296 to branch-1
> 
>
> Key: HBASE-18029
> URL: https://issues.apache.org/jira/browse/HBASE-18029
> Project: HBase
>  Issue Type: Improvement
>  Components: regionserver
>Reporter: Duo Zhang
>




--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (HBASE-18029) Backport HBASE-15296 to branch-1

2017-05-11 Thread Andrew Purtell (JIRA) 

[ 
https://issues.apache.org/jira/browse/HBASE-18029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16006858#comment-16006858
 ] 

Andrew Purtell commented on HBASE-18029:


bq.  I suggest that we make interfaces for both StoreFile, StoreFileReader and 
StoreFileWriter if we really need to expose them to CP. The work should be done 
before 2.0 release.

+1

This is what we did for Region (versus HRegion). If we mean to export the 
internal types for usage (instead of simply opaque types to pass through as 
parameters) then we should make them supported interfaces.

> Backport HBASE-15296 to branch-1
> 
>
> Key: HBASE-18029
> URL: https://issues.apache.org/jira/browse/HBASE-18029
> Project: HBase
>  Issue Type: Improvement
>  Components: regionserver
>Reporter: Duo Zhang
>




--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (HBASE-18029) Backport HBASE-15296 to branch-1

2017-05-11 Thread Duo Zhang (JIRA) 

[ 
https://issues.apache.org/jira/browse/HBASE-18029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16006176#comment-16006176
 ] 

Duo Zhang commented on HBASE-18029:
---

[~appy] I found it is really hard to keep compatibility here. The problem is 
that, StoreFile.Reader is an instance of StoreFileReader, but StoreFileReader 
is not an instance of StoreFile.Reader. The StoreFile is also marked as 
IA.LimitedPrivate(without IS annotation...) so if we want to also keep 
compatibility then we must still declare the return value of createReader as 
StoreFile.Reader. So here comes the problem, we add new methods in coprocessor 
which return a StoreFileReader instead of StoreFile.Reader. These methods will 
be called in StoreFile.Reader. If a user implements the new method and returns 
a StoreFileReader, how can we cast it to a StoreFile.Reader instance?

And in fact, most of our IA.LimitPrivate(CP) classes are declared as 
IS.Evolving which means it is allowed to break the compatibility between minor 
releases. But practically, it is not a good idea to break compatibility between 
minor releases. It is a pain for the downstream projects. I think we need to 
revisit the IS annotations.

And for the StoreFile related classes, I think a big problem is that, we expose 
a class instead of an interface to CP, and the class is not only a data 
strcuture. I suggest that we make interfaces for both StoreFile, 
StoreFileReader and StoreFileWriter if we really need to expose them to CP. The 
work should be done before 2.0 release.

What do you think? [~stack] [~apurtell] [~lhofhansl].

Thanks.

> Backport HBASE-15296 to branch-1
> 
>
> Key: HBASE-18029
> URL: https://issues.apache.org/jira/browse/HBASE-18029
> Project: HBase
>  Issue Type: Improvement
>  Components: regionserver
>Reporter: Duo Zhang
>




--
This message was sent by Atlassian JIRA
(v6.3.15#6346)