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

Update operation causes duplicate entries #1092

Closed
Srdjan-V opened this issue Dec 25, 2024 · 11 comments
Closed

Update operation causes duplicate entries #1092

Srdjan-V opened this issue Dec 25, 2024 · 11 comments

Comments

@Srdjan-V
Copy link

The entity is using the nitrite id annotation, in this case the object is not getting removed, and its not updated concurrently. The tags get updated with a lock on the id.

The update op insert if absent is used.

Image

    public void updateUserItemTags(SKU sku, TagGenerator generator) {
        Lock lock = userItemsMutex.get(sku);
        lock.lock();
        try {
            appManager.database().items().get(sku).ifPresent(item -> {
                updateUserItemTags(item, generator);
            });
        } finally {
            lock.unlock();
        }
    }

I am also unable to remove the duplicated objects

I don't have a way of replicating this since it happens randomly.

Version: 4.3.0
Java: 21-jbr
Database: MvStore

@anidotnet
Copy link
Contributor

From your code it is not clear how you have configured the database or what does your entity looks like.

Could you please share the nitrite specific code involved? A reproducible code would be greatly appreciated.

@anidotnet
Copy link
Contributor

On a closer look on your code, are you trying to call the same function recursively while holding the lock? If my understanding is correct, you are creating a deadlock itself in your code, which has nothing to do with nitrite itself.

Remove the lock and try once.

@Srdjan-V
Copy link
Author

Nitrite config:

    protected Nitrite build(Configs configs) throws Exception {
        var path = configs
                .paths()
                .database()
                .toAbsolutePath();

        FileUtils.createParentDirectories(path.toFile());
        MVStoreModule storeModule = MVStoreModule.withConfig()
                .filePath(path.toFile())
                .compress(true)
                .build();

        JacksonMapper jacksonMapper = new JacksonMapper() {
            @Override protected <Target> Target convertFromDocument(Document source, Class<Target> type) {
                try {
                    return super.convertFromDocument(source, type);
                } catch (IllegalArgumentException iae) {
                    log.warn("Unable to convert from document, source: {}, type {}", source, type, iae);
                    return null;
                }
            }
        };
        for (Module jacksonModule : List.of(
                new ParameterNamesModule(),
                new Jdk8Module(),
                new JavaTimeModule())) {
            jacksonMapper.registerJacksonModule(jacksonModule);
        }

        return Nitrite.builder()
                .loadModule(storeModule)
                .loadModule(NitriteModule.module(jacksonMapper))
                .openOrCreate();
}

This piece of code can get called concurrently to update the tags. I'm locking this on the sku/id because multiple things can change/add tags for the entity at the same time.

    public void updateUserItemTags(SKU sku, TagGenerator generator) {
        Lock lock = userItemsMutex.get(sku);
        lock.lock();
        try {
            appManager.database().items().get(sku).ifPresent(UserItemEntity item -> {
                updateUserItemTags(item, generator);
            });
        } finally {
            lock.unlock();
        }
    }

    public void updateUserItemTags(UserItemEntity item, TagGenerator generator) {
        Lock lock = userItemsMutex.get(item.getSku());
        lock.lock();
        try {
            item.setTags(generator.generateTags(this, item));
            appManager.database().items().upsert(item);
        } finally {
            lock.unlock();
        }
    }

I'm using wrapper classes for db collections, in this case the key is a string and the upsert method is just the update method but insert if absent set to true

    public void upsert(OBJ obj) {
        repository.update(obj, true);
    }
@Entity("UserItems")
@Data
@AllArgsConstructor
@NoArgsConstructor
public class UserItemEntity implements TagQuery, SkuEntity {
    @Id
    private String id;

    private List<Tag> tags = new ObjectArrayList<>();
}

On a closer look on your code, are you trying to call the same function recursively while holding the lock?

I'm not, I have implemented this because of some race condition with the update of the tags that I had.

As for replicating the issue, I don't know what causes this, the duplication happens after some time seemingly at random, or perhaps when I initialize a load on the db.

@anidotnet
Copy link
Contributor

I don't see any problem with the nitrite configuration on a quick look. If the sole purpose of the lock is to update nitrite only, I'll say remove the lock and test, because nitrite has its own locking mechanism to prevent race conditions.

Also, in your code you are using the same mutex for both methods. The lock in second method is unnecessary and is trying to acquire lock from an already locked mutex. Either use different mutex for two methods or better don't use any lock if you are just updating nitrite.

@Srdjan-V
Copy link
Author

Srdjan-V commented Dec 26, 2024

I have looked into the database debug logs, and I found this
2024-12-25-3.database.gz

Image

1735151350020 -> Wed Dec 25 19:29:10 CET 2024
1735188683625 -> Thu Dec 26 05:51:23 CET 2024

[19:29:10] [TF2-Trader-platform-thread-4/DEBUG] [nitrite]: Updating document with id [1869517534225264640]NO₂ in UserItems
[19:29:10] [TF2-Trader-platform-thread-4/DEBUG] [nitrite]: Processed document with id [1869517534225264640]NO₂
[19:29:10] [TF2-Trader-platform-thread-4/DEBUG] [nitrite]: Updated document with id [1869517534225264640]NO₂ in UserItems

-----------------------------------------------------------------------------------------------------------

[19:29:44] [TF2-Trader-platform-thread-5/DEBUG] [nitrite]: Updating document with id [1869517534225264640]NO₂ in UserItems
[19:29:44] [TF2-Trader-platform-thread-5/DEBUG] [nitrite]: Processed document with id [1869517534225264640]NO₂
[19:29:44] [TF2-Trader-platform-thread-5/DEBUG] [nitrite]: Updated document with id [1869517534225264640]NO₂ in UserItems

-----------------------------------------------------------------------------------------------------------
It was not removed between these logs
-----------------------------------------------------------------------------------------------------------

[19:35:53] [BotListeningManager-76561199482936139/DEBUG] [nitrite]: No documents found for update in UserItems
[19:35:53] [BotListeningManager-76561199482936139/DEBUG] [nitrite]: Total 1 document(s) to be inserted in UserItems
[19:35:53] [BotListeningManager-76561199482936139/DEBUG] [nitrite]: Processed document with id: [1871988251355815936]NO₂
[19:35:53] [BotListeningManager-76561199482936139/DEBUG] [nitrite]: Inserting processed document with id [1871988251355815936]NO₂
[19:35:53] [BotListeningManager-76561199482936139/DEBUG] [nitrite]: Alerting event listeners for action : Insert in UserItems
[19:35:53] [BotListeningManager-76561199482936139/DEBUG] [nitrite]: Returning write result WriteResultImpl(nitriteIds=[[1871988251355815936]NO₂]) for collection UserItems

@Srdjan-V
Copy link
Author

Srdjan-V commented Dec 26, 2024

From my testing, I think that it's db corruption that causes the duplicates. So I ended up switching to rocks db, if I see the same behavior I will report it here.

@Srdjan-V
Copy link
Author

I have not observed the same issue I had with mv store, but I have found another.

There is a race condition that can happen when you clear a collection and update it right after.

java.lang.ClassCastException: class java.lang.Object cannot be cast to class java.util.List (java.lang.Object and java.util.List are in module java.base of loader 'bootstrap')
	at org.dizitart.no2.index.SingleFieldIndex.addIndexElement(SingleFieldIndex.java:145) ~[nitrite-4.3.0.jar:?]
	at org.dizitart.no2.index.SingleFieldIndex.write(SingleFieldIndex.java:73) ~[nitrite-4.3.0.jar:?]
	at org.dizitart.no2.index.ComparableIndexer.writeIndexEntry(ComparableIndexer.java:70) ~[nitrite-4.3.0.jar:?]
	at org.dizitart.no2.collection.operation.DocumentIndexWriter.writeIndexEntryInternal(DocumentIndexWriter.java:97) ~[nitrite-4.3.0.jar:?]
	at org.dizitart.no2.collection.operation.DocumentIndexWriter.writeIndexEntry(DocumentIndexWriter.java:50) ~[nitrite-4.3.0.jar:?]
	at org.dizitart.no2.collection.operation.WriteOperations.insert(WriteOperations.java:99) ~[nitrite-4.3.0.jar:?]
	at org.dizitart.no2.collection.operation.WriteOperations.update(WriteOperations.java:204) ~[nitrite-4.3.0.jar:?]
	at org.dizitart.no2.collection.operation.CollectionOperations.update(CollectionOperations.java:102) ~[nitrite-4.3.0.jar:?]
	at org.dizitart.no2.collection.DefaultNitriteCollection.update(DefaultNitriteCollection.java:125) ~[nitrite-4.3.0.jar:?]
	at org.dizitart.no2.repository.DefaultObjectRepository.update(DefaultObjectRepository.java:129) ~[nitrite-4.3.0.jar:?]
	at org.dizitart.no2.repository.DefaultObjectRepository.update(DefaultObjectRepository.java:118) ~[nitrite-4.3.0.jar:?]

This is probably because of this

NitriteMap<?, ?> indexMap = nitriteStore.openMap(indexMapName, Object.class, Object.class);

@Srdjan-V
Copy link
Author

Also while looking thought the code I found that you're using concurrent maps, but are not using them in a thread safe way, making them useless. Is this intentional?

nitriteMapRegistry = new ConcurrentHashMap<>();
nitriteRTreeMapRegistry = new ConcurrentHashMap<>();

if (nitriteMapRegistry.containsKey(mapName)) {
RocksDBMap<Key, Value> nitriteMap = (RocksDBMap<Key, Value>) nitriteMapRegistry.get(mapName);
if (UnknownType.class.equals(nitriteMap.getKeyType())) {
nitriteMap.setKeyType(keyType);
}
if (UnknownType.class.equals(nitriteMap.getValueType())) {
nitriteMap.setValueType(valueType);
}
return nitriteMap;
} else {
NitriteMap<Key, Value> nitriteMap = new RocksDBMap<>(mapName, this, this.reference, keyType, valueType);
nitriteMapRegistry.put(mapName, nitriteMap);
return nitriteMap;
}

@anidotnet
Copy link
Contributor

Could you please raise a separate issue for this and provide some reproducible code?

@Srdjan-V
Copy link
Author

It's not a problem, but im still wondering if i should make 2 issues or just for the race condition?

@anidotnet
Copy link
Contributor

For the race condition please.

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

No branches or pull requests

2 participants