milvus-io / milvus

A cloud-native vector database, storage for next generation AI applications
https://milvus.io
Apache License 2.0
31.05k stars 2.95k forks source link

[Bug]: two collections iterators should not share the same checkpoint file #37109

Open yanliang567 opened 1 month ago

yanliang567 commented 1 month ago

Is there an existing issue for this?

Environment

- Milvus version: master-20241023-80d48f1e-amd64
- Deployment mode(standalone or cluster): standalone
- MQ type(rocksmq, pulsar or kafka):    
- SDK version(e.g. pymilvus v2.0.0rc2): pymilvus-2.5.0rc103

Current Behavior

2 collections' iterators can share the same checkpoint file

Expected Behavior

shall not share the cp files between collections

Steps To Reproduce

1. create 2 collections with the same schema
2. query iterator 1 on collectionA with cp_file_1
3. iterator1.next() for a few times
4. query iterator 2 on collectionB with the same cp_file_1
5. iterator2.next()    --expected fail with error msg

Milvus Log

No response

Anything else?

run the test below:

 # 1. initialize with data
        collection_w = self.init_collection_general(prefix, True)[0]
        collection_w2 = self.init_collection_general(prefix, True)[0]

        # 2. call a new query iterator and iterator for some times
        batch_size = 150
        iterator_cp_file = f"/tmp/it_{collection_w.name}_cp"
        iterator = collection_w.query_iterator(batch_size=batch_size // 2, iterator_cp_file=iterator_cp_file)[0]
        iter_times = 0
        first_iter_times = ct.default_nb // batch_size // 2 // 2  # only iterate half of the data for the 1st time
        while iter_times < first_iter_times:
            iter_times += 1
            res = iterator.next()
            if len(res) == 0:
                iterator.close()
                assert False, f"The iterator ends before {first_iter_times} times iterators: iter_times: {iter_times}"
                break

        # 3. query iterator on the second collection with the same checkpoint file
        iterator2 = collection_w2.query_iterator(batch_size=batch_size, iterator_cp_file=iterator_cp_file)[0]
        print(iterator2.next())
yanliang567 commented 1 month ago

/assign @MrPresent-Han /unassign

xiaofan-luan commented 1 month ago

I don't recoomend to store it in a file. We do we simply return user a proto file and whether this file is persisted should be decided by user

xiaofan-luan commented 1 month ago

We shouldn't persist this anyway

MrPresent-Han commented 1 month ago

This is really decided by users, if they want to persist, then pass the cp file path into the iterator, if not, just skip this parameter

xiaofan-luan commented 1 month ago

This is really decided by users, if they want to persist, then pass the cp file path into the iterator, if not, just skip this parameter

why do we just get a proto and decide where to persistent or bypass to the another layer by themselves

MrPresent-Han commented 3 weeks ago

@xiaofan-luan has some reasonable considerations about this feature, we can release it to users for the time-being and wait for some responses. @yanliang567

MrPresent-Han commented 3 weeks ago

/unassign