Mailing-List: contact dev-help@hbase.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hbase.apache.org
MIME-Version: 1.0
In-Reply-To: <2737c77b.47a.150e32ebdd7.Coremail.wangyongqiang0617@163.com>
References: <2bd50b93.499d.150e092cde6.Coremail.wangyongqiang0617@163.com>
	<CALte62wwhGhv8NpRHfX82hmHc_26XvjzLpPTD9UJj1Wu_ERcww@mail.gmail.com>
	<2737c77b.47a.150e32ebdd7.Coremail.wangyongqiang0617@163.com>
Date: Sat, 7 Nov 2015 11:47:26 -0800
Message-ID: 
 <CALte62yjzixW3K4AgD5-JfdTRq5bUKEAP=Ri08cSS4MiC3gSAQ@mail.gmail.com>
Subject: Re: Re: improvements on snapshot
From: Ted Yu <yuzhihong@gmail.com>
To: "dev@hbase.apache.org" <dev@hbase.apache.org>
Content-Type: multipart/alternative; boundary=001a1149262c2e241a0523f8a171

--001a1149262c2e241a0523f8a171
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Current behavior for creating duplicate snapshot is:

hbase(main):005:0> snapshot 'IntegrationTestBigLinkedList', 'snap1'

ERROR: org.apache.hadoop.hbase.snapshot.SnapshotExistsException: Snapshot
'snap1' already stored on the filesystem.
at
org.apache.hadoop.hbase.master.snapshot.SnapshotManager.takeSnapshot(Snapsh=
otManager.java:530)
at
org.apache.hadoop.hbase.master.MasterRpcServices.snapshot(MasterRpcServices=
.java:1275)
at
org.apache.hadoop.hbase.protobuf.generated.MasterProtos$MasterService$2.cal=
lBlockingMethod(MasterProtos.java:51123)
at org.apache.hadoop.hbase.ipc.RpcServer.call(RpcServer.java:2114)

bq. before we create a snopshot of a table, we can check if there is a
snopshot with same name

Suppose snapshot name has been taken, are you going to try with different
name blindly ?
The second attempt may receive SnapshotExistsException, right ?

Better approach is to use list_snapshots command to obtain existing
snapshots so that your attempt is more guided.

Cheers

On Sat, Nov 7, 2015 at 10:21 AM, WangYQ <wangyongqiang0617@163.com> wrote:

> 1. i download the src code from:
> http://mirrors.hust.edu.cn/apache/hbase/0.98.15/
>     class: org.apache.hadoop.hbase.master.snapshot.SnapshotManager
>    method:   public void restoreSnapshot(SnapshotDescription reqSnapshot)
> throws IOException
>    line: 725
>   code:
>
>
>       if (cpHost !=3D null) {
>         cpHost.postRestoreSnapshot(reqSnapshot, snapshotTableDesc);
>       }
>     } else {
>       HTableDescriptor htd =3D
> RestoreSnapshotHelper.cloneTableSchema(snapshotTableDesc, tableName);
>       if (cpHost !=3D null) {
>         cpHost.preCloneSnapshot(reqSnapshot, htd);
>       }
>       cloneSnapshot(fsSnapshot, htd);
>       LOG.info("Clone snapshot=3D" + fsSnapshot.getName() + " as table=3D=
" +
> tableName);
>
>
>       if (cpHost !=3D null) {
>         cpHost.postCloneSnapshot(reqSnapshot, htd);
>       }
>
>      i think the clone:
>       HTableDescriptor htd =3D
> RestoreSnapshotHelper.cloneTableSchema(snapshotTableDesc, tableName);
>       can be removed.
>
>
> 2. this one is similar to the one in HBaseAdmin.getTableDecsriptor
> 3.  add method 'isSnapShotExists' in HBaseAdmin to help us to know if
> there is a snopshot on HDFS, just like HBaseAdmin.tableExists,
> before we create a snopshot of a table, we can check if there is a
> snopshot with same name
> 4. simplify the code
> version: hbase 0.98.15
> class: org.apache.hadoop.hbase.master.cleaner.CleanerChore
> method:private boolean checkAndDeleteFiles(List<FileStatus> files)
>     line: 235-243
>     code:
>
>
>       // trace which cleaner is holding on to each file
>       if (LOG.isTraceEnabled()) {
>         ImmutableSet<FileStatus> filteredFileSet =3D
> ImmutableSet.copyOf(filteredFiles);
>         for (FileStatus file : deletableValidFiles) {
>           if (!filteredFileSet.contains(file)) {
>             LOG.trace(file.getPath() + " is not deletable according to:" =
+
> cleaner);
>           }
>         }
>       }
>      i think these code can use set's difference operation instead, which
> is more straightforward
>
>
> At 2015-11-07 21:10:21, "Ted Yu" <yuzhihong@gmail.com> wrote:
> >For #1, I took a look at current 0.98 code but didn't find it.
> >Can you double check and tell us the updated line number ?
> >
> >For #2, I think it makes sense - considering there may be large number o=
f
> >snapshots in the cluster.
> >
> >For #3, can you tell us the use case for the new API ?
> >
> >Cheers
> >
> >On Fri, Nov 6, 2015 at 10:12 PM, =E7=8E=8B=E5=8B=87=E5=BC=BA <wangyongqi=
ang0617@163.com> wrote:
> >
> >> the version is hbase0.98.10, find some improvements on snapshot:
> >> 1. in class SnapshotManager, line 725, we make a clone of
> >> HTableDescriptor,  the clone is not so need to make
> >> 2 class HBaseAdmin, method
> >>     public void restoreSnapshot(final String snapshotName, boolean
> >> takeFailSafeSnapshot)
> >> {
> >>     TableName tableName =3D null;
> >>     for (SnapshotDescription snapshotInfo: listSnapshots()) {
> >>       if (snapshotInfo.getName().equals(snapshotName)) {
> >>         tableName =3D TableName.valueOf(snapshotInfo.getTable());
> >>         break;
> >>       }
> >>     }
> >> ......
> >> }
> >>   must get all snapshot from master and then get the one we need, can
> >> improve this, master just return the snapshot client needed
> >>
> >>
> >> 3. there should better has a method 'isSnapShotExists' in HBaseAdmin
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
>

--001a1149262c2e241a0523f8a171--