Uploaded image for project: 'HBase'
  1. HBase
  2. HBASE-18811

Make filter IA.LimitedPrivate and IS.Stable

    Details

    • Type: Task
    • Status: Reopened
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: 2.0.0-alpha-4
    • Component/s: None
    • Labels:
      None
    • Release Note:
      Hide
      The list of changed filters is shown below:
      # ColumnCountGetFilter
      # ColumnPaginationFilter
      # ColumnPrefixFilter
      # ColumnRangeFilter
      # CompareFilter
      # DependentColumnFilter
      # FamilyFilter
      # Filter
      # FilterBase
      # FilterList
      # FirstKeyOnlyFilter
      # FirstKeyValueMatchingQualifiersFilter
      # FuzzyRowFilter
      # InclusiveStopFilter
      # KeyOnlyFilter
      # MultiRowRangeFilter
      # MultipleColumnPrefixFilter
      # PageFilter
      # ParseFilter
      # PrefixFilter
      # QualifierFilter
      # RandomRowFilter
      # RowFilter
      # SingleColumnValueExcludeFilter
      # SingleColumnValueFilter
      # SkipFilter
      # TimestampsFilter
      # ValueFilter
      # WhileMatchFilter
      Show
      The list of changed filters is shown below: # ColumnCountGetFilter # ColumnPaginationFilter # ColumnPrefixFilter # ColumnRangeFilter # CompareFilter # DependentColumnFilter # FamilyFilter # Filter # FilterBase # FilterList # FirstKeyOnlyFilter # FirstKeyValueMatchingQualifiersFilter # FuzzyRowFilter # InclusiveStopFilter # KeyOnlyFilter # MultiRowRangeFilter # MultipleColumnPrefixFilter # PageFilter # ParseFilter # PrefixFilter # QualifierFilter # RandomRowFilter # RowFilter # SingleColumnValueExcludeFilter # SingleColumnValueFilter # SkipFilter # TimestampsFilter # ValueFilter # WhileMatchFilter

      Description

      We have many powerful callback functions to help user to build amazing application/services. The most of functions are declared as IA.LimitedPrivate excluding the filters. As i see it, the IA.LimitedPrivate will make the improvement of filter more flexible. Also, we can introduce more server-side components to filters. In conclusion, we should consider adding the limited private level for filter.

      1. HBASE-18811.v0.patch
        36 kB
        Chia-Ping Tsai

        Issue Links

          Activity

          Hide
          chia7712 Chia-Ping Tsai added a comment -

          The filters in production files are changed to IA.LimitedPrivate from IA.Public.

          Show
          chia7712 Chia-Ping Tsai added a comment - The filters in production files are changed to IA.LimitedPrivate from IA.Public.
          Hide
          anoop.hbase Anoop Sam John added a comment -

          Filters are used at client end along with Scan/Get right? WHy to change from Public? As per BC rules, the Limited private is less stricter than Public. So am not getting why we should reduce the BC agreement of the Filters. Can u explain pls?

          Show
          anoop.hbase Anoop Sam John added a comment - Filters are used at client end along with Scan/Get right? WHy to change from Public? As per BC rules, the Limited private is less stricter than Public. So am not getting why we should reduce the BC agreement of the Filters. Can u explain pls?
          Hide
          chia7712 Chia-Ping Tsai added a comment -

          We should not expose the inner class/logic to Public Client. Right? All methods in observer are embedded in our server-side logic, so we make them IA.LimitedPrivate. All methods in filters are also embedded in our server-side logic, but we expose them as Public Client. The BC we need to guarantee is that the built-in filters must works as before (class name and arguments), even if we make drastic changes to it's impl.

          Show
          chia7712 Chia-Ping Tsai added a comment - We should not expose the inner class/logic to Public Client. Right? All methods in observer are embedded in our server-side logic, so we make them IA.LimitedPrivate. All methods in filters are also embedded in our server-side logic, but we expose them as Public Client. The BC we need to guarantee is that the built-in filters must works as before (class name and arguments), even if we make drastic changes to it's impl.
          Hide
          abhishek.chouhan Abhishek Singh Chouhan added a comment -

          Since filters are something that is directly used in the most basic client APIs scans/gets shouldn't we atleast have an interface (eg. filter which is currently an abstract class) that should be IA.Public and then we can keep the implementations LimitedPrivate? That way we stick to the basic behavioral guarantees and api contract and can change the implementations internally as per our needs on the server side.

          Show
          abhishek.chouhan Abhishek Singh Chouhan added a comment - Since filters are something that is directly used in the most basic client APIs scans/gets shouldn't we atleast have an interface (eg. filter which is currently an abstract class) that should be IA.Public and then we can keep the implementations LimitedPrivate? That way we stick to the basic behavioral guarantees and api contract and can change the implementations internally as per our needs on the server side.
          Hide
          chia7712 Chia-Ping Tsai added a comment -

          shouldn't we atleast have an interface (eg. filter which is currently an abstract class) that should be IA.Public and then we can keep the implementations LimitedPrivate?

          Good idea. Nevertheless, that still expose many methods to Public Client. The Get/Scan should carry only the POJO such as row(byte[]) and flag(boolean), hence the Filter is not a acceptable class. We can introduce an POJO class to carry the data used to build a filter. The POJO class looks like this:

          IA.Public
          interface FilterProvider {
            byte[] toByteArray();
            static FilterProvider parseFrom(byte[]);
            Filter create(); 
          }
          
          class RowFilterProvider implements FilterProvider {
            RowFilterProvider(CompareOperator, ByteArrayComparable);
            Filter create() {
              return new RowFilter();
            }
          }
          

          Also, we will deprecate xxxFilter in Scan and suggest user to use xxxFilterProvider.

          Show
          chia7712 Chia-Ping Tsai added a comment - shouldn't we atleast have an interface (eg. filter which is currently an abstract class) that should be IA.Public and then we can keep the implementations LimitedPrivate? Good idea. Nevertheless, that still expose many methods to Public Client. The Get/Scan should carry only the POJO such as row(byte[]) and flag(boolean), hence the Filter is not a acceptable class. We can introduce an POJO class to carry the data used to build a filter. The POJO class looks like this: IA.Public interface FilterProvider { byte [] toByteArray(); static FilterProvider parseFrom( byte []); Filter create(); } class RowFilterProvider implements FilterProvider { RowFilterProvider(CompareOperator, ByteArrayComparable); Filter create() { return new RowFilter(); } } Also, we will deprecate xxxFilter in Scan and suggest user to use xxxFilterProvider .
          Hide
          davelatham Dave Latham added a comment -

          The idea here is that Filters should no longer be part of the client API, but move to being internals, like coprocessors?
          As one HBase user, we'd be sorry to see this. We do have custom filters deployed, but have shied away from coprocessors due to the lesser degree of stability and higher degree of complexity.

          I think a discussion on the dev list would probably be good before making this change.

          Show
          davelatham Dave Latham added a comment - The idea here is that Filters should no longer be part of the client API, but move to being internals, like coprocessors? As one HBase user, we'd be sorry to see this. We do have custom filters deployed, but have shied away from coprocessors due to the lesser degree of stability and higher degree of complexity. I think a discussion on the dev list would probably be good before making this change.
          Hide
          chia7712 Chia-Ping Tsai added a comment -

          Dave Latham Thanks for the response!!

          I think a discussion on the dev list would probably be good before making this change.

          Got it. I will resolve it as Later. I had already opened a thread in dev list.

          but have shied away from coprocessors due to the lesser degree of stability and higher degree of complexity.

          Let us move the discussion to dev list. see you then.

          Show
          chia7712 Chia-Ping Tsai added a comment - Dave Latham Thanks for the response!! I think a discussion on the dev list would probably be good before making this change. Got it. I will resolve it as Later . I had already opened a thread in dev list. but have shied away from coprocessors due to the lesser degree of stability and higher degree of complexity. Let us move the discussion to dev list. see you then.
          Hide
          davelatham Dave Latham added a comment -

          Thanks, Chia-Ping Tsai. Sorry I missed the thread. I understand there's a tradeoff, and if other folks think this is best changed, then I can understand. I see the mighty stack thinks it oughta happen. I'll chime in on the dev list, but no need to resolve yet.

          Show
          davelatham Dave Latham added a comment - Thanks, Chia-Ping Tsai . Sorry I missed the thread. I understand there's a tradeoff, and if other folks think this is best changed, then I can understand. I see the mighty stack thinks it oughta happen. I'll chime in on the dev list, but no need to resolve yet.

            People

            • Assignee:
              chia7712 Chia-Ping Tsai
              Reporter:
              chia7712 Chia-Ping Tsai
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:

                Development