leaking transactions in ConsistentKeyLocker


Madhan Neethiraj <mad...@...>
 

Hi Boxuan,

 

I created issue #2255, and pull request #2256 with the fix for the issue. Can you please review?

 

Thanks,

Madhan

 

 

 

 

From: <janusg...@...> on behalf of BO XUAN LI <li...@...>
Reply-To: <janusg...@...>
Date: Saturday, November 21, 2020 at 7:23 PM
To: <janusg...@...>
Subject: Re: leaking transactions in ConsistentKeyLocker

 

Looks cool, can you submit an issue and a PR?

 

Regards,

Boxuan



On Nov 21, 2020, at 10:50 AM, Madhan Neethiraj <mad...@...> wrote:

 

Hi Boxuan,

 

Attached patch has updated unit test to detect leaked transactions in ConsistentKeyLocker. 14 of 28 tests in ConsistentKeyLockerTest fail due to this issue.

 

Once this bug is confirmed by the group, I can open an issue and submit a pull request with the fix.

 

Thanks,

Madhan

 

From: <janusgra...@...> on behalf of BO XUAN LI <libo...@...>
Reply-To: <janusgra...@...>
Date: Monday, November 9, 2020 at 7:29 AM
To: <janusgra...@...>
Subject: Re: leaking transactions in ConsistentKeyLocker

 

Hi Madhan,

 

I am not familiar with this part thus cannot say much on this, but do you have any example (or even better, unit test) that can reproduce the issue?

 

Regards,

Boxuan




On Nov 8, 2020, at 9:37 AM, 'Madhan Neethiraj' via JanusGraph users <janusgra...@...> wrote:

 

Transactions created in ConsistentKeyLocker.overrideTimestamp() are neither committed or rolled back by the callers. This results in leaked transactions. This issue (of leaked transactions) was seen while implementing a custom StoreManager. Addressing the issue required updating ConsistentKeyLocker, to commit the created transactions.

 

The fix is straight forward, and I have patches for master and v0.5.1. Can someone please review and either confirm the issue or suggest alternate (to avoid leaked transactions)?


Relevant code from ConsistentKeyLocker.java is given below.

 

Thanks,

Madhan

 

  private WriteResult tryWriteLockOnce(StaticBuffer key, StaticBuffer del, StoreTransaction txh) {

    ...

    try {

      final StoreTransaction newTx = overrideTimestamp(txh, writeTimer.getStartTime());

      store.mutate(key, Collections.singletonList(newLockEntry),

          null == del ? KeyColumnValueStore.NO_DELETIONS : Collections.singletonList(del), newTx);

      } catch (BackendException e) {

        ...

  }

 

  private WriteResult tryDeleteLockOnce(StaticBuffer key, StaticBuffer col, StoreTransaction txh) {

    ...

    try {

      final StoreTransaction newTx = overrideTimestamp(txh, delTimer.getStartTime());

      store.mutate(key, ImmutableList.of(), Collections.singletonList(col), newTx);

    } catch (BackendException e) {

      ...

  }

 

  protected void deleteSingleLock(KeyColumn kc, ConsistentKeyLockStatus ls, StoreTransaction tx) {

    ...

    try {

      StoreTransaction newTx = overrideTimestamp(tx, times.getTime());

      store.mutate(serializer.toLockKey(kc.getKey(), kc.getColumn()), ImmutableList.of(), deletions, newTx);

      return;

    } catch (TemporaryBackendException e) {

      ...

  }

 

  private StoreTransaction overrideTimestamp(final StoreTransaction tx, final Instant commitTime) throws BackendException {
    StandardBaseTransactionConfig newCfg = new StandardBaseTransactionConfig.Builder(tx.getConfiguration()).commitTime(commitTime).build();
    return manager.beginTransaction(newCfg);
  }

 

-- 
You received this message because you are subscribed to the Google Groups "JanusGraph users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to janusgra...@....
To view this discussion on the web visit https://groups.google.com/d/msgid/janusgraph-users/6e5f4ef5-42a6-446a-818f-1aa14b128994n%40googlegroups.com.

 

-- 
You received this message because you are subscribed to the Google Groups "JanusGraph users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to janusgra...@....
To view this discussion on the web visit https://groups.google.com/d/msgid/janusgraph-users/94112DC5-67D3-4E0A-A895-759F08DA8346%40connect.hku.hk.


 

-- 
You received this message because you are subscribed to the Google Groups "JanusGraph users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to 
janusgra...@....
To view this discussion on the web visit 
https://groups.google.com/d/msgid/janusgraph-users/B94C23DA-1484-4E50-BD0A-C39B760FAE4C%40apache.org.
<unit-test-updates-to-detect-transaction-leak-in-Cons.patch>

 

--
You received this message because you are subscribed to the Google Groups "JanusGraph users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to janusgra...@....
To view this discussion on the web visit https://groups.google.com/d/msgid/janusgraph-users/6D009E7F-F69B-4057-BA16-AC11A13D21BE%40connect.hku.hk.


BO XUAN LI <libo...@...>
 

Looks cool, can you submit an issue and a PR?

Regards,
Boxuan

On Nov 21, 2020, at 10:50 AM, Madhan Neethiraj <mad...@...> wrote:

Hi Boxuan,
 
Attached patch has updated unit test to detect leaked transactions in ConsistentKeyLocker. 14 of 28 tests in ConsistentKeyLockerTest fail due to this issue.
 
Once this bug is confirmed by the group, I can open an issue and submit a pull request with the fix.
 
Thanks,
Madhan
 
From: <janusgra...@...> on behalf of BO XUAN LI <libo...@...>
Reply-To: <janusgra...@...>
Date: Monday, November 9, 2020 at 7:29 AM
To: <janusgra...@...>
Subject: Re: leaking transactions in ConsistentKeyLocker
 
Hi Madhan,
 
I am not familiar with this part thus cannot say much on this, but do you have any example (or even better, unit test) that can reproduce the issue?
 
Regards,
Boxuan


On Nov 8, 2020, at 9:37 AM, 'Madhan Neethiraj' via JanusGraph users <janusgra...@...> wrote:
 
Transactions created in ConsistentKeyLocker.overrideTimestamp() are neither committed or rolled back by the callers. This results in leaked transactions. This issue (of leaked transactions) was seen while implementing a custom StoreManager. Addressing the issue required updating ConsistentKeyLocker, to commit the created transactions.
 
The fix is straight forward, and I have patches for master and v0.5.1. Can someone please review and either confirm the issue or suggest alternate (to avoid leaked transactions)?

Relevant code from ConsistentKeyLocker.java is given below.
 
Thanks,
Madhan
 
  private WriteResult tryWriteLockOnce(StaticBuffer key, StaticBuffer del, StoreTransaction txh) {
    ...
    try {
      final StoreTransaction newTx = overrideTimestamp(txh, writeTimer.getStartTime());
      store.mutate(key, Collections.singletonList(newLockEntry),
          null == del ? KeyColumnValueStore.NO_DELETIONS : Collections.singletonList(del), newTx);
      } catch (BackendException e) {
        ...
  }
 
  private WriteResult tryDeleteLockOnce(StaticBuffer key, StaticBuffer col, StoreTransaction txh) {
    ...
    try {
      final StoreTransaction newTx = overrideTimestamp(txh, delTimer.getStartTime());
      store.mutate(key, ImmutableList.of(), Collections.singletonList(col), newTx);
    } catch (BackendException e) {
      ...
  }
 
  protected void deleteSingleLock(KeyColumn kc, ConsistentKeyLockStatus ls, StoreTransaction tx) {
    ...
    try {
      StoreTransaction newTx = overrideTimestamp(tx, times.getTime());
      store.mutate(serializer.toLockKey(kc.getKey(), kc.getColumn()), ImmutableList.of(), deletions, newTx);
      return;
    } catch (TemporaryBackendException e) {
      ...
  }
 
  private StoreTransaction overrideTimestamp(final StoreTransaction tx, final Instant commitTime) throws BackendException {
    StandardBaseTransactionConfig newCfg = new StandardBaseTransactionConfig.Builder(tx.getConfiguration()).commitTime(commitTime).build();
    return manager.beginTransaction(newCfg);
  }
 
-- 
You received this message because you are subscribed to the Google Groups "JanusGraph users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to janusgra...@....
To view this discussion on the web visit https://groups.google.com/d/msgid/janusgraph-users/6e5f4ef5-42a6-446a-818f-1aa14b128994n%40googlegroups.com.
 
-- 
You received this message because you are subscribed to the Google Groups "JanusGraph users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to janusgra...@....
To view this discussion on the web visit https://groups.google.com/d/msgid/janusgraph-users/94112DC5-67D3-4E0A-A895-759F08DA8346%40connect.hku.hk.


-- 
You received this message because you are subscribed to the Google Groups "JanusGraph users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to janusgra...@....
To view this discussion on the web visit https://groups.google.com/d/msgid/janusgraph-users/B94C23DA-1484-4E50-BD0A-C39B760FAE4C%40apache.org.
<unit-test-updates-to-detect-transaction-leak-in-Cons.patch>


Madhan Neethiraj <mad...@...>
 

Hi Boxuan,

 

Attached patch has updated unit test to detect leaked transactions in ConsistentKeyLocker. 14 of 28 tests in ConsistentKeyLockerTest fail due to this issue.

 

Once this bug is confirmed by the group, I can open an issue and submit a pull request with the fix.

 

Thanks,

Madhan

 

From: <janusg...@...> on behalf of BO XUAN LI <li...@...>
Reply-To: <janusg...@...>
Date: Monday, November 9, 2020 at 7:29 AM
To: <janusg...@...>
Subject: Re: leaking transactions in ConsistentKeyLocker

 

Hi Madhan,

 

I am not familiar with this part thus cannot say much on this, but do you have any example (or even better, unit test) that can reproduce the issue?

 

Regards,

Boxuan



On Nov 8, 2020, at 9:37 AM, 'Madhan Neethiraj' via JanusGraph users <janusgra...@...> wrote:

 

Transactions created in ConsistentKeyLocker.overrideTimestamp() are neither committed or rolled back by the callers. This results in leaked transactions. This issue (of leaked transactions) was seen while implementing a custom StoreManager. Addressing the issue required updating ConsistentKeyLocker, to commit the created transactions.

 

The fix is straight forward, and I have patches for master and v0.5.1. Can someone please review and either confirm the issue or suggest alternate (to avoid leaked transactions)?


Relevant code from ConsistentKeyLocker.java is given below.

 

Thanks,

Madhan

 

  private WriteResult tryWriteLockOnce(StaticBuffer key, StaticBuffer del, StoreTransaction txh) {

    ...

    try {

      final StoreTransaction newTx = overrideTimestamp(txh, writeTimer.getStartTime());

      store.mutate(key, Collections.singletonList(newLockEntry),

          null == del ? KeyColumnValueStore.NO_DELETIONS : Collections.singletonList(del), newTx);

      } catch (BackendException e) {

        ...

  }

 

  private WriteResult tryDeleteLockOnce(StaticBuffer key, StaticBuffer col, StoreTransaction txh) {

    ...

    try {

      final StoreTransaction newTx = overrideTimestamp(txh, delTimer.getStartTime());

      store.mutate(key, ImmutableList.of(), Collections.singletonList(col), newTx);

    } catch (BackendException e) {

      ...

  }

 

  protected void deleteSingleLock(KeyColumn kc, ConsistentKeyLockStatus ls, StoreTransaction tx) {

    ...

    try {

      StoreTransaction newTx = overrideTimestamp(tx, times.getTime());

      store.mutate(serializer.toLockKey(kc.getKey(), kc.getColumn()), ImmutableList.of(), deletions, newTx);

      return;

    } catch (TemporaryBackendException e) {

      ...

  }

 

  private StoreTransaction overrideTimestamp(final StoreTransaction tx, final Instant commitTime) throws BackendException {
    StandardBaseTransactionConfig newCfg = new StandardBaseTransactionConfig.Builder(tx.getConfiguration()).commitTime(commitTime).build();
    return manager.beginTransaction(newCfg);
  }

 

--
You received this message because you are subscribed to the Google Groups "JanusGraph users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to janusgra...@....
To view this discussion on the web visit https://groups.google.com/d/msgid/janusgraph-users/6e5f4ef5-42a6-446a-818f-1aa14b128994n%40googlegroups.com.

 

--
You received this message because you are subscribed to the Google Groups "JanusGraph users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to janusgra...@....
To view this discussion on the web visit https://groups.google.com/d/msgid/janusgraph-users/94112DC5-67D3-4E0A-A895-759F08DA8346%40connect.hku.hk.


BO XUAN LI <libo...@...>
 

Hi Madhan,

I am not familiar with this part thus cannot say much on this, but do you have any example (or even better, unit test) that can reproduce the issue?

Regards,
Boxuan

On Nov 8, 2020, at 9:37 AM, 'Madhan Neethiraj' via JanusGraph users <janusgra...@...> wrote:

Transactions created in ConsistentKeyLocker.overrideTimestamp() are neither committed or rolled back by the callers. This results in leaked transactions. This issue (of leaked transactions) was seen while implementing a custom StoreManager. Addressing the issue required updating ConsistentKeyLocker, to commit the created transactions.

The fix is straight forward, and I have patches for master and v0.5.1. Can someone please review and either confirm the issue or suggest alternate (to avoid leaked transactions)?

Relevant code from ConsistentKeyLocker.java is given below.

Thanks,
Madhan

  private WriteResult tryWriteLockOnce(StaticBuffer key, StaticBuffer del, StoreTransaction txh) {
    ...
    try {
      final StoreTransaction newTx = overrideTimestamp(txh, writeTimer.getStartTime());
      store.mutate(key, Collections.singletonList(newLockEntry),
          null == del ? KeyColumnValueStore.NO_DELETIONS : Collections.singletonList(del), newTx);
      } catch (BackendException e) {
        ...
  }

  private WriteResult tryDeleteLockOnce(StaticBuffer key, StaticBuffer col, StoreTransaction txh) {
    ...
    try {
      final StoreTransaction newTx = overrideTimestamp(txh, delTimer.getStartTime());
      store.mutate(key, ImmutableList.of(), Collections.singletonList(col), newTx);
    } catch (BackendException e) {
      ...
  }

  protected void deleteSingleLock(KeyColumn kc, ConsistentKeyLockStatus ls, StoreTransaction tx) {
    ...
    try {
      StoreTransaction newTx = overrideTimestamp(tx, times.getTime());
      store.mutate(serializer.toLockKey(kc.getKey(), kc.getColumn()), ImmutableList.of(), deletions, newTx);
      return;
    } catch (TemporaryBackendException e) {
      ...
  }

  private StoreTransaction overrideTimestamp(final StoreTransaction tx, final Instant commitTime) throws BackendException {
    StandardBaseTransactionConfig newCfg = new StandardBaseTransactionConfig.Builder(tx.getConfiguration()).commitTime(commitTime).build();
    return manager.beginTransaction(newCfg);
  }


--
You received this message because you are subscribed to the Google Groups "JanusGraph users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to janusgra...@....
To view this discussion on the web visit https://groups.google.com/d/msgid/janusgraph-users/6e5f4ef5-42a6-446a-818f-1aa14b128994n%40googlegroups.com.


Madhan Neethiraj <mneet...@...>
 

Transactions created in ConsistentKeyLocker.overrideTimestamp() are neither committed or rolled back by the callers. This results in leaked transactions. This issue (of leaked transactions) was seen while implementing a custom StoreManager. Addressing the issue required updating ConsistentKeyLocker, to commit the created transactions.

The fix is straight forward, and I have patches for master and v0.5.1. Can someone please review and either confirm the issue or suggest alternate (to avoid leaked transactions)?

Relevant code from ConsistentKeyLocker.java is given below.

Thanks,
Madhan

  private WriteResult tryWriteLockOnce(StaticBuffer key, StaticBuffer del, StoreTransaction txh) {
    ...
    try {
      final StoreTransaction newTx = overrideTimestamp(txh, writeTimer.getStartTime());
      store.mutate(key, Collections.singletonList(newLockEntry),
          null == del ? KeyColumnValueStore.NO_DELETIONS : Collections.singletonList(del), newTx);
      } catch (BackendException e) {
        ...
  }

  private WriteResult tryDeleteLockOnce(StaticBuffer key, StaticBuffer col, StoreTransaction txh) {
    ...
    try {
      final StoreTransaction newTx = overrideTimestamp(txh, delTimer.getStartTime());
      store.mutate(key, ImmutableList.of(), Collections.singletonList(col), newTx);
    } catch (BackendException e) {
      ...
  }

  protected void deleteSingleLock(KeyColumn kc, ConsistentKeyLockStatus ls, StoreTransaction tx) {
    ...
    try {
      StoreTransaction newTx = overrideTimestamp(tx, times.getTime());
      store.mutate(serializer.toLockKey(kc.getKey(), kc.getColumn()), ImmutableList.of(), deletions, newTx);
      return;
    } catch (TemporaryBackendException e) {
      ...
  }

  private StoreTransaction overrideTimestamp(final StoreTransaction tx, final Instant commitTime) throws BackendException {
    StandardBaseTransactionConfig newCfg = new StandardBaseTransactionConfig.Builder(tx.getConfiguration()).commitTime(commitTime).build();
    return manager.beginTransaction(newCfg);
  }