dev.morphia.Datastore Thread Safety

[MKaramavrov]

Discussed in #2802

^{Originally posted by AmitKuGarg December 18, 2023}
Describe the bug
When dev.morphia.Datastore shared across thread, I get Two entities have been mapped using the same discriminator value.

To Reproduce
Steps to reproduce the behavior:

1. Create a sample collection and create the datastore.
   Datastore datastore = Morphia.createDatastore(mongo, dbName);
   datastore.ensureIndexes();
   if(this.packages != null){
   for (String packageString : packages) {
   datastore.getMapper().map(packageString);
   }
   }

2. Create a runnable class and pass the data store and id parameter. In run method load the data from the collection using id and datastore.
   class Task implements Runnable {
   private String id;
   private Datastore mongoDatastore;

   Task(String id, Datastore mongoDatastore) {
   this.id = id;
   this.mongoDatastore = mongoDatastore;
   }

   @OverRide
   public void run() {

    Sample result = mongoDatastore.find(Sample.class).filter(Filters.eq("_id", id)).first();        
    if(result == null){
        System.out.println("Result is null");
    }else {
        System.out.println(result.toString());
    }
   

   }
   }

3. Write main method, create ExecutorService and try to load different documents.
   ExecutorService executor = Executors.newFixedThreadPool(10);
   executor.execute(new Task("ABC12345", mongoDatastore));
   executor.execute(new Task("ABC123456", mongoDatastore));

I get Two entities have been mapped using the same discriminator value.

Try two scenario

1. When ids are valid id from collection
2. Try some invalid ids where collection return null.

[MKaramavrov]

In our project, we faced the same issue with lazy mapping of the entities during runtime. Version, which we currently use is 2.4.11.

[evanchooly]

I'm hesitant to do too much thread-safety work here as this is a very common method to call. A better fix, on your end, is to not rely on lazy mapping (especially as that is going away) and explicitly map your classes/packages up front. This should eliminate your issues. Please try pre-mapping and see if your issue goes away. If it doesn't, we can talk about this PR.

[MKaramavrov]

That was already a discussion here: #2802
Would be nice to have a fix at least for 2.4.x version, because we've just migrated from version 1.6.x to 2.4.11 and we are currently trying to preregister every entity on startup, but there is conceptual issue, because morphia does not enforce preregistration and during code change someone can miss some entity which will lead to hard to catch and reproduce issue.

[MKaramavrov]

@evanchooly Could we please consider fixing the issue at least for 2.4.x versions?
Not sure that we will have too much thread safety, as we are checking for presence of existing model before making it synchronized:

if (existing != null) {
            return existing;
        }

[aksdb]

We also run into this issue during our migration from 1.6 to 2.4.

We also pre-register our entities during startup, but it looks like getEntityModel on a "root"-entity doesn't cause the embedded entities to be registered, which then still happens on the first save() call which then can lead to concurrency issues.

Mapping all entities upfront wouldn't really work either, since each datasource has its own mapper with Morphia 2.4. If we map all entities on all datasources, that seems quite inefficient (especially since most entities wouldn't be used on all datasources).

I don't quite understand why no read/write mutex is used for this, if this is so critical.