Author: jimk Date: Sat Jul 14 17:26:15 2007 New Revision: 556359 URL: http://svn.apache.org/viewvc?view=rev&rev=556359 Log: HADOOP-1614 [hbase] HClient does not protect itself from simultaneous updates
Added: lucene/hadoop/trunk/src/contrib/hbase/src/test/org/apache/hadoop/hbase/TestMultipleUpdates.java Modified: lucene/hadoop/trunk/src/contrib/hbase/CHANGES.txt lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/HClient.java Modified: lucene/hadoop/trunk/src/contrib/hbase/CHANGES.txt URL: http://svn.apache.org/viewvc/lucene/hadoop/trunk/src/contrib/hbase/CHANGES.txt?view=diff&rev=556359&r1=556358&r2=556359 ============================================================================== --- lucene/hadoop/trunk/src/contrib/hbase/CHANGES.txt (original) +++ lucene/hadoop/trunk/src/contrib/hbase/CHANGES.txt Sat Jul 14 17:26:15 2007 @@ -62,3 +62,4 @@ 38. HADOOP-1574 Concurrent creates of a table named 'X' all succeed 39. HADOOP-1581 Un-openable tablename bug 40. HADOOP-1607 [shell] Clear screen command (Edward Yoon via Stack) + 41. HADOOP-1614 [hbase] HClient does not protect itself from simultaneous updates Modified: lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/HClient.java URL: http://svn.apache.org/viewvc/lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/HClient.java?view=diff&rev=556359&r1=556358&r2=556359 ============================================================================== --- lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/HClient.java (original) +++ lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/HClient.java Sat Jul 14 17:26:15 2007 @@ -59,6 +59,7 @@ int numRetries; private HMasterInterface master; private final Configuration conf; + private long currentLockId; private Class<? extends HRegionInterface> serverInterfaceClass; /* @@ -120,6 +121,7 @@ */ public HClient(Configuration conf) { this.conf = conf; + this.currentLockId = -1; this.pause = conf.getLong("hbase.client.pause", 30 * 1000); this.numRetries = conf.getInt("hbase.client.retries.number", 5); @@ -608,6 +610,9 @@ if(tableName == null || tableName.getLength() == 0) { throw new IllegalArgumentException("table name cannot be null or zero length"); } + if(this.currentLockId != -1) { + throw new IllegalStateException("update in progress"); + } this.tableServers = getTableServers(tableName); } @@ -1325,8 +1330,10 @@ * @return Row lockid. * @throws IOException */ - public long startUpdate(final Text row) throws IOException { - long lockid = -1; + public synchronized long startUpdate(final Text row) throws IOException { + if(this.currentLockId != -1) { + throw new IllegalStateException("update in progress"); + } for(int tries = 0; tries < numRetries; tries++) { IOException e = null; RegionLocation info = getRegionLocation(row); @@ -1334,7 +1341,7 @@ currentServer = getHRegionConnection(info.serverAddress); currentRegion = info.regionInfo.regionName; clientid = rand.nextLong(); - lockid = currentServer.startUpdate(currentRegion, clientid, row); + this.currentLockId = currentServer.startUpdate(currentRegion, clientid, row); break; } catch (IOException ex) { @@ -1359,7 +1366,7 @@ throw e; } } - return lockid; + return this.currentLockId; } /** @@ -1372,6 +1379,9 @@ * @throws IOException */ public void put(long lockid, Text column, byte val[]) throws IOException { + if(lockid != this.currentLockId) { + throw new IllegalArgumentException("invalid lockid"); + } try { this.currentServer.put(this.currentRegion, this.clientid, lockid, column, val); @@ -1398,6 +1408,9 @@ * @throws IOException */ public void delete(long lockid, Text column) throws IOException { + if(lockid != this.currentLockId) { + throw new IllegalArgumentException("invalid lockid"); + } try { this.currentServer.delete(this.currentRegion, this.clientid, lockid, column); @@ -1423,6 +1436,9 @@ * @throws IOException */ public void abort(long lockid) throws IOException { + if(lockid != this.currentLockId) { + throw new IllegalArgumentException("invalid lockid"); + } try { this.currentServer.abort(this.currentRegion, this.clientid, lockid); } catch(IOException e) { @@ -1432,6 +1448,8 @@ e = RemoteExceptionHandler.decodeRemoteException((RemoteException) e); } throw e; + } finally { + this.currentLockId = -1; } } @@ -1453,6 +1471,9 @@ * @throws IOException */ public void commit(long lockid, long timestamp) throws IOException { + if(lockid != this.currentLockId) { + throw new IllegalArgumentException("invalid lockid"); + } try { this.currentServer.commit(this.currentRegion, this.clientid, lockid, timestamp); @@ -1464,6 +1485,8 @@ e = RemoteExceptionHandler.decodeRemoteException((RemoteException) e); } throw e; + } finally { + this.currentLockId = -1; } } Added: lucene/hadoop/trunk/src/contrib/hbase/src/test/org/apache/hadoop/hbase/TestMultipleUpdates.java URL: http://svn.apache.org/viewvc/lucene/hadoop/trunk/src/contrib/hbase/src/test/org/apache/hadoop/hbase/TestMultipleUpdates.java?view=auto&rev=556359 ============================================================================== --- lucene/hadoop/trunk/src/contrib/hbase/src/test/org/apache/hadoop/hbase/TestMultipleUpdates.java (added) +++ lucene/hadoop/trunk/src/contrib/hbase/src/test/org/apache/hadoop/hbase/TestMultipleUpdates.java Sat Jul 14 17:26:15 2007 @@ -0,0 +1,119 @@ +/** + * Copyright 2007 The Apache Software Foundation + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase; + +import org.apache.hadoop.io.Text; + +/** + * Tests that HClient protects against multiple updates + */ +public class TestMultipleUpdates extends HBaseClusterTestCase { + private static final String CONTENTS_STR = "contents:"; + private static final Text CONTENTS = new Text(CONTENTS_STR); + private static final byte[] value = { 1, 2, 3, 4 }; + + private HTableDescriptor desc = null; + private HClient client = null; + + /** + * [EMAIL PROTECTED] + */ + @Override + public void setUp() throws Exception { + super.setUp(); + this.client = new HClient(conf); + this.desc = new HTableDescriptor("test"); + desc.addFamily(new HColumnDescriptor(CONTENTS_STR)); + try { + client.createTable(desc); + client.openTable(desc.getName()); + + } catch (Exception e) { + e.printStackTrace(); + fail(); + } + } + + /** the test */ + public void testMultipleUpdates() { + try { + long lockid = client.startUpdate(new Text("row1")); + + try { + long lockid2 = client.startUpdate(new Text("row2")); + throw new Exception("second startUpdate returned lock id " + lockid2); + + } catch (IllegalStateException i) { + // expected + } + + try { + client.openTable(HConstants.META_TABLE_NAME); + + } catch (IllegalStateException i) { + // expected + } + + long invalidid = 42; + + try { + client.put(invalidid, CONTENTS, value); + + } catch (IllegalArgumentException i) { + // expected + } + + try { + client.delete(invalidid, CONTENTS); + + } catch (IllegalArgumentException i) { + // expected + } + + try { + client.put(invalidid, CONTENTS, value); + + } catch (IllegalArgumentException i) { + // expected + } + + try { + client.abort(invalidid); + + } catch (IllegalArgumentException i) { + // expected + } + + try { + client.commit(invalidid); + + } catch (IllegalArgumentException i) { + // expected + } + + client.abort(lockid); + + } catch (Exception e) { + System.err.println("unexpected exception"); + e.printStackTrace(); + fail(); + } + } +}