[jira] [Updated] (GEODE-10121) Transactional Redis commands are not actually transactional

[Anthony Baker (Jira)]

 [ 
https://issues.apache.org/jira/browse/GEODE-10121?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Anthony Baker updated GEODE-10121:
--
Labels: pull-request-available redis-triage  (was: blocks-1.15.0​ 
pull-request-available)

> Transactional Redis commands are not actually transactional
> ---
>
> Key: GEODE-10121
> URL: https://issues.apache.org/jira/browse/GEODE-10121
> Project: Geode
>  Issue Type: Bug
>  Components: redis
>Affects Versions: 1.15.0
>Reporter: Donal Evans
>Priority: Major
>  Labels: pull-request-available, redis-triage
>
> The following test (if added to MSetDUnitTest) is intended to show that MSET 
> is transactional in nature by having the final key to be set throw an 
> exception, which should roll back the previous set values:
> {code:java}
>   public static final String THROWING_REDIS_STRING_EXCEPTION = "to be 
> ignored";
>   
>   @Test
>   public void mset_isTransactional() {
> IgnoredException.addIgnoredException(THROWING_REDIS_STRING_EXCEPTION);
> String hashTag = "{" + clusterStartUp.getKeyOnServer("tag", 1) + "}";
> String key1 = hashTag + "key1";
> String value1 = "value1";
> jedis.set(key1, value1);
> String key2 = hashTag + "key2";
> String value2 = "value2";
> jedis.set(key2, value2);
> String throwingRedisStringKey = hashTag + "ThrowingRedisString";
> String throwingStringValue = "ThrowingRedisStringValue";
> // Put a test version of RedisString directly into the region that throws 
> if the multi-argument set() is called on it
> clusterStartUp.getMember(1).invoke(() -> {
>   RedisKey throwingKey = new 
> RedisKey(throwingRedisStringKey.getBytes(StandardCharsets.UTF_8));
>   ThrowingRedisString throwingRedisString = new ThrowingRedisString();
>   
> throwingRedisString.set(throwingStringValue.getBytes(StandardCharsets.UTF_8));
>   
> ClusterStartupRule.getCache().getRegion(DEFAULT_REDIS_REGION_NAME).put(throwingKey,
>  throwingRedisString);
> });
> String newValue = "should_not_be_set";
> assertThatThrownBy(() -> jedis.mset(key1, newValue, key2, newValue, 
> throwingRedisStringKey, newValue)).hasMessage(SERVER_ERROR_MESSAGE);
> 
> assertThat(jedis.get(key1)).isEqualTo(value1);
> assertThat(jedis.get(key2)).isEqualTo(value2);
> 
> assertThat(jedis.get(throwingRedisStringKey)).isEqualTo(throwingStringValue);
> IgnoredException.removeAllExpectedExceptions();
>   }
>   static class ThrowingRedisString extends RedisString {
> @Override
> public void set(Region region, RedisKey key, byte[] 
> newValue,
> SetOptions options) {
>   throw new RuntimeException(THROWING_REDIS_STRING_EXCEPTION);
> }
>   }{code}
> This test fails due to {{key1}} having its value set to {{newValue}} despite 
> the fact that MSET is supposed to be atomic.
> Given the similar implementations each command uses, it is very likely that 
> MSETNX and SMOVE also show the same behaviour.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

[jira] [Updated] (GEODE-10121) Transactional Redis commands are not actually transactional

[ASF GitHub Bot (Jira)]

 [ 
https://issues.apache.org/jira/browse/GEODE-10121?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

ASF GitHub Bot updated GEODE-10121:
---
Labels: blocks-1.15.0​ pull-request-available  (was: blocks-1.15.0​)

> Transactional Redis commands are not actually transactional
> ---
>
> Key: GEODE-10121
> URL: https://issues.apache.org/jira/browse/GEODE-10121
> Project: Geode
>  Issue Type: Bug
>  Components: redis
>Affects Versions: 1.15.0
>Reporter: Donal Evans
>Priority: Major
>  Labels: blocks-1.15.0​, pull-request-available
>
> The following test (if added to MSetDUnitTest) is intended to show that MSET 
> is transactional in nature by having the final key to be set throw an 
> exception, which should roll back the previous set values:
> {code:java}
>   public static final String THROWING_REDIS_STRING_EXCEPTION = "to be 
> ignored";
>   
>   @Test
>   public void mset_isTransactional() {
> IgnoredException.addIgnoredException(THROWING_REDIS_STRING_EXCEPTION);
> String hashTag = "{" + clusterStartUp.getKeyOnServer("tag", 1) + "}";
> String key1 = hashTag + "key1";
> String value1 = "value1";
> jedis.set(key1, value1);
> String key2 = hashTag + "key2";
> String value2 = "value2";
> jedis.set(key2, value2);
> String throwingRedisStringKey = hashTag + "ThrowingRedisString";
> String throwingStringValue = "ThrowingRedisStringValue";
> // Put a test version of RedisString directly into the region that throws 
> if the multi-argument set() is called on it
> clusterStartUp.getMember(1).invoke(() -> {
>   RedisKey throwingKey = new 
> RedisKey(throwingRedisStringKey.getBytes(StandardCharsets.UTF_8));
>   ThrowingRedisString throwingRedisString = new ThrowingRedisString();
>   
> throwingRedisString.set(throwingStringValue.getBytes(StandardCharsets.UTF_8));
>   
> ClusterStartupRule.getCache().getRegion(DEFAULT_REDIS_REGION_NAME).put(throwingKey,
>  throwingRedisString);
> });
> String newValue = "should_not_be_set";
> assertThatThrownBy(() -> jedis.mset(key1, newValue, key2, newValue, 
> throwingRedisStringKey, newValue)).hasMessage(SERVER_ERROR_MESSAGE);
> 
> assertThat(jedis.get(key1)).isEqualTo(value1);
> assertThat(jedis.get(key2)).isEqualTo(value2);
> 
> assertThat(jedis.get(throwingRedisStringKey)).isEqualTo(throwingStringValue);
> IgnoredException.removeAllExpectedExceptions();
>   }
>   static class ThrowingRedisString extends RedisString {
> @Override
> public void set(Region region, RedisKey key, byte[] 
> newValue,
> SetOptions options) {
>   throw new RuntimeException(THROWING_REDIS_STRING_EXCEPTION);
> }
>   }{code}
> This test fails due to {{key1}} having its value set to {{newValue}} despite 
> the fact that MSET is supposed to be atomic.
> Given the similar implementations each command uses, it is very likely that 
> MSETNX and SMOVE also show the same behaviour.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

[jira] [Updated] (GEODE-10121) Transactional Redis commands are not actually transactional

[Donal Evans (Jira)]

 [ 
https://issues.apache.org/jira/browse/GEODE-10121?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Donal Evans updated GEODE-10121:

Description: 
The following test (if added to MSetDUnitTest) is intended to show that MSET is 
transactional in nature by having the final key to be set throw an exception, 
which should roll back the previous set values:
{code:java}
  public static final String THROWING_REDIS_STRING_EXCEPTION = "to be ignored";
  
  @Test
  public void mset_isTransactional() {
IgnoredException.addIgnoredException(THROWING_REDIS_STRING_EXCEPTION);
String hashTag = "{" + clusterStartUp.getKeyOnServer("tag", 1) + "}";

String key1 = hashTag + "key1";
String value1 = "value1";
jedis.set(key1, value1);
String key2 = hashTag + "key2";
String value2 = "value2";
jedis.set(key2, value2);

String throwingRedisStringKey = hashTag + "ThrowingRedisString";
String throwingStringValue = "ThrowingRedisStringValue";

// Put a test version of RedisString directly into the region that throws 
if the multi-argument set() is called on it
clusterStartUp.getMember(1).invoke(() -> {
  RedisKey throwingKey = new 
RedisKey(throwingRedisStringKey.getBytes(StandardCharsets.UTF_8));
  ThrowingRedisString throwingRedisString = new ThrowingRedisString();
  
throwingRedisString.set(throwingStringValue.getBytes(StandardCharsets.UTF_8));
  
ClusterStartupRule.getCache().getRegion(DEFAULT_REDIS_REGION_NAME).put(throwingKey,
 throwingRedisString);
});

String newValue = "should_not_be_set";

assertThatThrownBy(() -> jedis.mset(key1, newValue, key2, newValue, 
throwingRedisStringKey, newValue)).hasMessage(SERVER_ERROR_MESSAGE);

assertThat(jedis.get(key1)).isEqualTo(value1);
assertThat(jedis.get(key2)).isEqualTo(value2);

assertThat(jedis.get(throwingRedisStringKey)).isEqualTo(throwingStringValue);

IgnoredException.removeAllExpectedExceptions();
  }

  static class ThrowingRedisString extends RedisString {
@Override
public void set(Region region, RedisKey key, byte[] 
newValue,
SetOptions options) {
  throw new RuntimeException(THROWING_REDIS_STRING_EXCEPTION);
}
  }{code}
This test fails due to {{key1}} having its value set to {{newValue}} despite 
the fact that MSET is supposed to be atomic.

Given the similar implementations each command uses, it is very likely that 
MSETNX and SMOVE also show the same behaviour.

  was:
The following test (if added to MSetDUnitTest) is intended to show that MSET is 
transactional in nature by having the final key to be set throw an exception, 
which should roll back the previous set values:
{code:java}
  public static final String THROWING_REDIS_STRING_EXCEPTION = "to be ignored";
  
  @Test
  public void mset_isTransactional() {
IgnoredException.addIgnoredException(THROWING_REDIS_STRING_EXCEPTION);
String hashTag = "{" + clusterStartUp.getKeyOnServer("tag", 1) + "}";

String key1 = hashTag + "key1";
String value1 = "value1";
jedis.set(key1, value1);
String key2 = hashTag + "key2";
String value2 = "value2";
jedis.set(key2, value2);

String throwingRedisStringKey = hashTag + "ThrowingRedisString";
String throwingStringValue = "ThrowingRedisStringValue";

// Put a test version of RedisString directly into the region that throws 
if the multi-argument set() is called on it
clusterStartUp.getMember(1).invoke(() -> {
  RedisKey throwingKey = new 
RedisKey(throwingRedisStringKey.getBytes(StandardCharsets.UTF_8));
  ThrowingRedisString throwingRedisString = new ThrowingRedisString();
  
throwingRedisString.set(throwingStringValue.getBytes(StandardCharsets.UTF_8));
  
ClusterStartupRule.getCache().getRegion(DEFAULT_REDIS_REGION_NAME).put(throwingKey,
 throwingRedisString);
});

String newValue = "should_not_be_set";

assertThatThrownBy(() -> jedis.mset(key1, newValue, key2, newValue, 
throwingRedisStringKey, newValue)).hasMessage(SERVER_ERROR_MESSAGE);

assertThat(jedis.get(key1)).isEqualTo(value1);
assertThat(jedis.get(key2)).isEqualTo(value2);

assertThat(jedis.get(throwingRedisStringKey)).isEqualTo(throwingStringValue);

IgnoredException.removeAllExpectedExceptions();
  }

  static class ThrowingRedisString extends RedisString {
@Override
public void set(Region region, RedisKey key, byte[] 
newValue,
SetOptions options) {
  throw new RuntimeException(THROWING_REDIS_STRING_EXCEPTION);
}
  }{code}
This test fails due to {{key1}} having its value set to {{newValue}} despite 
the fact that MSET is supposed to be atomic.

Given the similar implementations each command uses, it is very likely that 
RENAME, MSETNX and SMOVE also show the same behaviour.


> Transactional Redis commands are not actually transactional
> ---

[jira] [Updated] (GEODE-10121) Transactional Redis commands are not actually transactional

[Donal Evans (Jira)]

 [ 
https://issues.apache.org/jira/browse/GEODE-10121?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Donal Evans updated GEODE-10121:

Labels: blocks-1.15.0​  (was: needsTriage)

> Transactional Redis commands are not actually transactional
> ---
>
> Key: GEODE-10121
> URL: https://issues.apache.org/jira/browse/GEODE-10121
> Project: Geode
>  Issue Type: Bug
>  Components: redis
>Affects Versions: 1.15.0, 1.16.0
>Reporter: Donal Evans
>Priority: Major
>  Labels: blocks-1.15.0​
>
> The following test (if added to MSetDUnitTest) is intended to show that MSET 
> is transactional in nature by having the final key to be set throw an 
> exception, which should roll back the previous set values:
> {code:java}
>   public static final String THROWING_REDIS_STRING_EXCEPTION = "to be 
> ignored";
>   
>   @Test
>   public void mset_isTransactional() {
> IgnoredException.addIgnoredException(THROWING_REDIS_STRING_EXCEPTION);
> String hashTag = "{" + clusterStartUp.getKeyOnServer("tag", 1) + "}";
> String key1 = hashTag + "key1";
> String value1 = "value1";
> jedis.set(key1, value1);
> String key2 = hashTag + "key2";
> String value2 = "value2";
> jedis.set(key2, value2);
> String throwingRedisStringKey = hashTag + "ThrowingRedisString";
> String throwingStringValue = "ThrowingRedisStringValue";
> // Put a test version of RedisString directly into the region that throws 
> if the multi-argument set() is called on it
> clusterStartUp.getMember(1).invoke(() -> {
>   RedisKey throwingKey = new 
> RedisKey(throwingRedisStringKey.getBytes(StandardCharsets.UTF_8));
>   ThrowingRedisString throwingRedisString = new ThrowingRedisString();
>   
> throwingRedisString.set(throwingStringValue.getBytes(StandardCharsets.UTF_8));
>   
> ClusterStartupRule.getCache().getRegion(DEFAULT_REDIS_REGION_NAME).put(throwingKey,
>  throwingRedisString);
> });
> String newValue = "should_not_be_set";
> assertThatThrownBy(() -> jedis.mset(key1, newValue, key2, newValue, 
> throwingRedisStringKey, newValue)).hasMessage(SERVER_ERROR_MESSAGE);
> 
> assertThat(jedis.get(key1)).isEqualTo(value1);
> assertThat(jedis.get(key2)).isEqualTo(value2);
> 
> assertThat(jedis.get(throwingRedisStringKey)).isEqualTo(throwingStringValue);
> IgnoredException.removeAllExpectedExceptions();
>   }
>   static class ThrowingRedisString extends RedisString {
> @Override
> public void set(Region region, RedisKey key, byte[] 
> newValue,
> SetOptions options) {
>   throw new RuntimeException(THROWING_REDIS_STRING_EXCEPTION);
> }
>   }{code}
> This test fails due to {{key1}} having its value set to {{newValue}} despite 
> the fact that MSET is supposed to be atomic.
> Given the similar implementations each command uses, it is very likely that 
> RENAME, MSETNX and SMOVE also show the same behaviour.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

[jira] [Updated] (GEODE-10121) Transactional Redis commands are not actually transactional

[Alexander Murmann (Jira)]

 [ 
https://issues.apache.org/jira/browse/GEODE-10121?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Alexander Murmann updated GEODE-10121:
--
Labels: needsTriage  (was: )

> Transactional Redis commands are not actually transactional
> ---
>
> Key: GEODE-10121
> URL: https://issues.apache.org/jira/browse/GEODE-10121
> Project: Geode
>  Issue Type: Bug
>  Components: redis
>Affects Versions: 1.15.0, 1.16.0
>Reporter: Donal Evans
>Priority: Major
>  Labels: needsTriage
>
> The following test (if added to MSetDUnitTest) is intended to show that MSET 
> is transactional in nature by having the final key to be set throw an 
> exception, which should roll back the previous set values:
> {code:java}
>   public static final String THROWING_REDIS_STRING_EXCEPTION = "to be 
> ignored";
>   
>   @Test
>   public void mset_isTransactional() {
> IgnoredException.addIgnoredException(THROWING_REDIS_STRING_EXCEPTION);
> String hashTag = "{" + clusterStartUp.getKeyOnServer("tag", 1) + "}";
> String key1 = hashTag + "key1";
> String value1 = "value1";
> jedis.set(key1, value1);
> String key2 = hashTag + "key2";
> String value2 = "value2";
> jedis.set(key2, value2);
> String throwingRedisStringKey = hashTag + "ThrowingRedisString";
> String throwingStringValue = "ThrowingRedisStringValue";
> // Put a test version of RedisString directly into the region that throws 
> if the multi-argument set() is called on it
> clusterStartUp.getMember(1).invoke(() -> {
>   RedisKey throwingKey = new 
> RedisKey(throwingRedisStringKey.getBytes(StandardCharsets.UTF_8));
>   ThrowingRedisString throwingRedisString = new ThrowingRedisString();
>   
> throwingRedisString.set(throwingStringValue.getBytes(StandardCharsets.UTF_8));
>   
> ClusterStartupRule.getCache().getRegion(DEFAULT_REDIS_REGION_NAME).put(throwingKey,
>  throwingRedisString);
> });
> String newValue = "should_not_be_set";
> assertThatThrownBy(() -> jedis.mset(key1, newValue, key2, newValue, 
> throwingRedisStringKey, newValue)).hasMessage(SERVER_ERROR_MESSAGE);
> 
> assertThat(jedis.get(key1)).isEqualTo(value1);
> assertThat(jedis.get(key2)).isEqualTo(value2);
> 
> assertThat(jedis.get(throwingRedisStringKey)).isEqualTo(throwingStringValue);
> IgnoredException.removeAllExpectedExceptions();
>   }
>   static class ThrowingRedisString extends RedisString {
> @Override
> public void set(Region region, RedisKey key, byte[] 
> newValue,
> SetOptions options) {
>   throw new RuntimeException(THROWING_REDIS_STRING_EXCEPTION);
> }
>   }{code}
> This test fails due to {{key1}} having its value set to {{newValue}} despite 
> the fact that MSET is supposed to be atomic.
> Given the similar implementations each command uses, it is very likely that 
> RENAME, MSETNX and SMOVE also show the same behaviour.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)