Mailing-List: contact dev-help@accumulo.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@accumulo.apache.org
Content-Type: multipart/alternative;
 boundary="===============2979599809511241137=="
MIME-Version: 1.0
Subject: Re: Review Request 25915: ACCUMULO-3135 Modify server-side
 table-operations
 to throw ThriftTableOperationException when table doesn't exist instead of
 ThriftSecurityException
From: "Josh Elser" <josh.elser@gmail.com>
To: keith@deenlo.com
Cc: "accumulo" <dev@accumulo.apache.org>, "Josh Elser" <josh.elser@gmail.com>
Date: Tue, 23 Sep 2014 02:12:01 -0000
Message-ID: <20140923021201.15006.2445@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Josh Elser" <noreply@reviews.apache.org>
References: <20140922234149.14999.63249@reviews.apache.org>
In-Reply-To: <20140922234149.14999.63249@reviews.apache.org>
Reply-To: "Josh Elser" <josh.elser@gmail.com>

--===============2979599809511241137==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25915/
-----------------------------------------------------------

(Updated Sept. 23, 2014, 2:12 a.m.)


Review request for accumulo and kturner.


Changes
-------

Described some (contrived) testing I did.


Bugs: ACCUMULO-3135
    https://issues.apache.org/jira/browse/ACCUMULO-3135


Repository: accumulo


Description
-------

The server-side implementations of the table operations typically
follow the pattern of: accept table name, get table id, check
permission, run table operation.

Fetching the table id does a (trusted) check of whether or not
the table that was requested to operate upon actually exists or
not (we don't want to blindly accept table IDs from users in most
cases). However, there is a race condition in which a table may
be deleted after we fetch the table ID and before we can check the
permissions for the user on said table.

SecurityOperation only throws ThriftSecurityExceptions. While this
makes sense in the context of the SecurityOperation class, we have
to translate a ThriftSecurityException for a nonexistent table
into a ThriftTableOperationException so that the client implementation
will throw a TableNotFoundException instead of an AccumuloSecurityException.


Diffs
-----

  server/src/main/java/org/apache/accumulo/server/master/Master.java 12f8fed 

Diff: https://reviews.apache.org/r/25915/diff/


Testing (updated)
-------

Ran unit tests so far. Modified the MasterClientServiceHandler to sleep before rename and clone, then used MiniAccumuloCluster to start a rename/clone and delete the source table before the operation could start. Then, I made sure that the exception coming back out of TableOperations was a TableNotFoundException, not an AccumuloSecurityException.


Thanks,

Josh Elser


--===============2979599809511241137==--