[jira] [Comment Edited] (HBASE-21645) Perform sanity check and disallow table creation/modification with region replication < 1

["Nihal Jain (JIRA)" <ji...@apache.org>]

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

Nihal Jain edited comment on HBASE-21645 at 1/7/19 7:29 PM:
------------------------------------------------------------

{quote}What will happed if replica config to 0? 
{quote}
No, it would create a table normally without any replica region, as we have the following check in {{RegionReplicaUtil.addReplicas()}}, which is called during {{CreateTableProcedure.addTableToMeta()}}. 

This check ensures we create replica regions, if replica count > 0

{code:java}
  public static List<RegionInfo> addReplicas(final TableDescriptor tableDescriptor,
      final List<RegionInfo> regions, int oldReplicaCount, int newReplicaCount) {
    if ((newReplicaCount - 1) <= 0) {
      return regions;
    }
    List<RegionInfo> hRegionInfos = new ArrayList<>((newReplicaCount) * regions.size());
    for (RegionInfo ri : regions) {
      if (RegionReplicaUtil.isDefaultReplica(ri) &&
        (!ri.isOffline() || (!ri.isSplit() && !ri.isSplitParent()))) {
        // region level replica index starts from 0. So if oldReplicaCount was 2 then the max replicaId for
        // the existing regions would be 1
        for (int j = oldReplicaCount; j < newReplicaCount; j++) {
          hRegionInfos.add(RegionReplicaUtil.getRegionInfoForReplica(ri, j));
        }
      }
    }
    hRegionInfos.addAll(regions);
    return hRegionInfos;
  }
{code}
{quote} And please add the table name to the message. 
{quote}
Since sanity check is called during create table, do we really need to mention table name? Also, none of the other exceptions are logging tablename. Should I address this? What do you think?
{quote}Why not use warnOrThrowExceptionForFailure to keep same pattern with other checks?
{quote}
We can definitely do that, it would also ensure same behaviour as previous versions, but I saw that for replication scope sanity check (See [HBASE-16419|https://issues.apache.org/jira/browse/HBASE-16419] ) we are purposely not following {{warnOrThrowExceptionForFailure}} pattern. Anyways, I am willing to do otherwise. What do you propose?

 


was (Author: nihaljain.cs):
{quote}What will happed if replica config to 0? 
{quote}
No, it would create a table with 1 region, as we have the following check in {{RegionReplicaUtil.addReplicas()}}, which is used during {{CreateTableProcedure.addTableToMeta(), which ensures we only return replica regions, if replica count > 1.}}
{code:java}
  public static List<RegionInfo> addReplicas(final TableDescriptor tableDescriptor,
      final List<RegionInfo> regions, int oldReplicaCount, int newReplicaCount) {
    if ((newReplicaCount - 1) <= 0) {
      return regions;
    }
    List<RegionInfo> hRegionInfos = new ArrayList<>((newReplicaCount) * regions.size());
    for (RegionInfo ri : regions) {
      if (RegionReplicaUtil.isDefaultReplica(ri) &&
        (!ri.isOffline() || (!ri.isSplit() && !ri.isSplitParent()))) {
        // region level replica index starts from 0. So if oldReplicaCount was 2 then the max replicaId for
        // the existing regions would be 1
        for (int j = oldReplicaCount; j < newReplicaCount; j++) {
          hRegionInfos.add(RegionReplicaUtil.getRegionInfoForReplica(ri, j));
        }
      }
    }
    hRegionInfos.addAll(regions);
    return hRegionInfos;
  }
{code}
{quote} And please add the table name to the message. 
{quote}
Since sanity check is called during create table, do we really need to mention table name? Also, none of the other exceptions are logging tablename. Should I address this? What do you think?
{quote}Why not use warnOrThrowExceptionForFailure to keep same pattern with other checks?
{quote}
We can definitely do that, it would also ensure same behaviour as previous versions, but I saw that for replication scope sanity check (See [HBASE-16419|https://issues.apache.org/jira/browse/HBASE-16419] ) we are purposely not following {{warnOrThrowExceptionForFailure}} pattern. Anyways, I am willing to do otherwise. What do you propose?

 

> Perform sanity check and disallow table creation/modification with region replication < 1
> -----------------------------------------------------------------------------------------
>
>                 Key: HBASE-21645
>                 URL: https://issues.apache.org/jira/browse/HBASE-21645
>             Project: HBase
>          Issue Type: Improvement
>    Affects Versions: 3.0.0, 1.5.0, 2.1.1, 2.1.2
>            Reporter: Nihal Jain
>            Assignee: Nihal Jain
>            Priority: Minor
>         Attachments: HBASE-21645.master.001.patch, HBASE-21645.master.001.patch
>
>
> We should perform sanity check and disallow table creation with region replication < 1 or modification of an existing table with new region replication value < 1.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)