[jira] [Commented] (HBASE-14465) Backport 'Allow rowlock to be reader/write' to branch-1

["James Taylor (JIRA)" <ji...@apache.org>]

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

James Taylor commented on HBASE-14465:
--------------------------------------

Question on this one, as there appear to be inconsistencies (at least in the Region javadoc and the HRegion implementation):
For HBase 1.2, here's the javadoc from the Region interface:
{code}
  /**
   * Tries to acquire a lock on the given row.
   * @param waitForLock if true, will block until the lock is available.
   *        Otherwise, just tries to obtain the lock and returns
   *        false if unavailable.
   * @return the row lock if acquired,
   *   null if waitForLock was false and the lock was not acquired
   * @throws IOException if waitForLock was true and the lock could not be acquired after waiting
   */
  RowLock getRowLock(byte[] row, boolean waitForLock) throws IOException;
{code}
But it seems that the meaning of the boolean has changed, as here's the implementation:
{code}
  /**
   *
   * Get a row lock for the specified row. All locks are reentrant.
   *
   * Before calling this function make sure that a region operation has already been
   * started (the calling thread has already acquired the region-close-guard lock).
   * @param row The row actions will be performed against
   * @param readLock is the lock reader or writer. True indicates that a non-exlcusive
   *                 lock is requested
   */
  public RowLock getRowLock(byte[] row, boolean readLock) throws IOException {
{code}

Then in HBase 0.98, HRegion still works the old way:
{code}
  /**
   * Tries to acquire a lock on the given row.
   * @param waitForLock if true, will block until the lock is available.
   *        Otherwise, just tries to obtain the lock and returns
   *        false if unavailable.
   * @return the row lock if acquired,
   *   null if waitForLock was false and the lock was not acquired
   * @throws IOException if waitForLock was true and the lock could not be acquired after waiting
   */
  public RowLock getRowLock(byte[] row, boolean waitForLock) throws IOException {
{code}

Keeping the same method signature but changing the meaning is a dangerous change IMHO. Perhaps creating a new method name would be prudent, [~stack], [~busbey]? Phoenix is using this API.

> Backport 'Allow rowlock to be reader/write' to branch-1
> -------------------------------------------------------
>
>                 Key: HBASE-14465
>                 URL: https://issues.apache.org/jira/browse/HBASE-14465
>             Project: HBase
>          Issue Type: Sub-task
>          Components: regionserver
>            Reporter: stack
>            Assignee: stack
>             Fix For: 1.2.0, 1.3.0
>
>         Attachments: 14465.branch-1.txt, 14465.branch-1.v2.txt, 14465.branch-1.v2.txt, 14465.branch-1.v2.txt, 14465.branch-1.v3.txt, 14465.branch-1.v3.txt, 14465.branch-1.v4.txt, 14465.branch-1.v4.txt, 14465.branch-1.v4.txt, 14465.branch-1.v5.txt, 14465.branch-1.v5.txt, 14465.branch-1.v9.txt, 14465.branch-1.v9.txt, 14465.branch-1.v9.txt, 14465.txt
>
>
> Backport to branch-1.
> You want this in 1.2 [~busbey]? Its cleanup and fixes a possible dataloss. On other hand, its a bit of refactoring.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)