Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guarantee stale reads from Map inside a transaction, when NearCache and delayed MapStore enabled, 3.7.1/3.7.2 #9248

Closed
raidan2 opened this issue Nov 12, 2016 · 8 comments

Comments

@raidan2
Copy link

@raidan2 raidan2 commented Nov 12, 2016

Hi,

If you have a map with NearCache enabled and a MapStore and your store implementation is a little bit clunky (or you have reasonable write delay) then you will get stale reads inside a transaction with simple condition: if you ever try to GET this key outside of transaction first.

Disabling NearCache solves the issue.

It looks fixed in master branch already but this could be reproduced in current 3.7.3 branch.
Is it possible to have a backport fix for 3.7x?

Test:

import com.hazelcast.config.Config;
import com.hazelcast.config.InMemoryFormat;
import com.hazelcast.config.MapStoreConfig;
import com.hazelcast.config.NearCacheConfig;
import com.hazelcast.core.Hazelcast;
import com.hazelcast.core.HazelcastInstance;
import com.hazelcast.core.MapStore;
import com.hazelcast.transaction.TransactionException;
import com.hazelcast.transaction.TransactionalTask;
import com.hazelcast.transaction.TransactionalTaskContext;
import org.junit.Assert;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

public class HazelcastTransactionsTest {
	private static final Logger LOGGER = LoggerFactory.getLogger(HazelcastTransactionsTest.class);

	private static final String MAP = "SAMPLE";

	static class SlowMapStore implements MapStore<Long, String> {
		private final Map<Long, String> backend = new ConcurrentHashMap<Long, String>();

		private final long saveSleep;

		public SlowMapStore() {
			this(100);
		}

		SlowMapStore(long saveSleep) {
			this.saveSleep = saveSleep;
		}

		private void sleep() {
			try {
				Thread.sleep(saveSleep);
			} catch (InterruptedException e) {
				e.printStackTrace();
			}
		}

		@Override
		public void store(Long key, String value) {
			sleep();
			LOGGER.info("Store {} = {}", key, value);
			backend.put(key, value);
		}

		@Override
		public void storeAll(Map<Long, String> map) {
			sleep();
			LOGGER.info("Store all {}", map.keySet());
			backend.putAll(map);
		}

		@Override
		public void delete(Long key) {
			sleep();
			LOGGER.info("Delete {}", key);
			backend.remove(key);
		}

		@Override
		public void deleteAll(Collection<Long> keys) {
			sleep();
			LOGGER.info("Delete all {}", keys);
			for (Long key : keys) {
				backend.remove(key);
			}
		}

		@Override
		public String load(Long key) {
			final String value = backend.get(key);
			LOGGER.info("Load {} = {}", key, value);
			return value;
		}

		@Override
		public Map<Long, String> loadAll(Collection<Long> keys) {
			LOGGER.info("Load all {}", keys);
			final Map<Long, String> result = new HashMap<Long, String>();
			for (Long key : keys) {
				final String value = backend.get(key);
				if (value != null) {
					result.put(key, value);
				}
			}
			return result;
		}

		@Override
		public Iterable<Long> loadAllKeys() {
			return new ArrayList<Long>(backend.keySet());
		}
	}

	@Test
	public void testTransactionInconsistentRead() throws InterruptedException {
		final Config config = new Config();

		config.getMapConfig(MAP)
				.setMapStoreConfig(new MapStoreConfig()
						.setClassName(SlowMapStore.class.getName())
						.setWriteBatchSize(512)
						.setInitialLoadMode(MapStoreConfig.InitialLoadMode.EAGER)
						.setWriteDelaySeconds(2))
				.setBackupCount(1)
				.setAsyncBackupCount(0)
				.setNearCacheConfig(new NearCacheConfig()
						.setInMemoryFormat(InMemoryFormat.OBJECT)
						.setCacheLocalEntries(true));

		final HazelcastInstance hz1 = Hazelcast.newHazelcastInstance(config);
		try {

			LOGGER.info("Transaction 1");

			Assert.assertNull(hz1.getMap(MAP).get(1L)); // This operation outside of transaction ruins everything

			hz1.executeTransaction(new TransactionalTask<Object>() {

				@Override
				public Object execute(TransactionalTaskContext context) throws TransactionException {
					context.getMap(MAP).set(1L, "Test1-1");
					return null;
				}
			});

			LOGGER.info("Transaction 2");
			hz1.executeTransaction(new TransactionalTask<Object>() {
				@Override
				public Object execute(TransactionalTaskContext context) throws TransactionException {
					Assert.assertEquals("Test1-1", context.getMap(MAP).get(1L)); // Find nothing!
					return null;
				}
			});

			LOGGER.info("Complete");
		} finally {
			hz1.shutdown();
		}
	}
}
@jerrinot jerrinot added this to the 3.8 milestone Nov 14, 2016
@jerrinot
Copy link
Contributor

@jerrinot jerrinot commented Nov 14, 2016

@Donnerbart can you please have a look? thanks!

@Donnerbart
Copy link
Contributor

@Donnerbart Donnerbart commented Nov 14, 2016

I have to look into this in detail, but my first guess would be that the invalidation unification by @ahmetmircik fixed this. But this is no candidate for a backport, since it changed the behavior of the invalidations on members too much (they are now executed asynchronously in the background instead of synchronously within the method call).

@jerrinot
Copy link
Contributor

@jerrinot jerrinot commented Dec 15, 2016

@raidan2 Unfortunately backport would break wire-level compatibility :(
We just released Hazelcast 3.8 Early Access. Hazelcast 3.8 Final is on the horizon!

@jerrinot jerrinot closed this Dec 15, 2016
@heliheli
Copy link

@heliheli heliheli commented Sep 28, 2017

The problem is reproducing in the last stable version (3.8.6)

@ahmetmircik
Copy link
Member

@ahmetmircik ahmetmircik commented Sep 28, 2017

@heliheli can you provide a reproducer for your case?

@heliheli
Copy link

@heliheli heliheli commented Sep 28, 2017

@ahmetmircik same test, from this issue

@ahmetmircik
Copy link
Member

@ahmetmircik ahmetmircik commented Sep 28, 2017

Reopening this one, seems it has never been addressed before.
Thanks for reporting it.

@ahmetmircik
Copy link
Member

@ahmetmircik ahmetmircik commented Oct 6, 2017

FYI: @heliheli now it should be fixed in current master(will be released in 3.9) and maintenance(will be released in 3.8.7):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants
You can’t perform that action at this time.