diff --git a/docs/write-routing-policy.md b/docs/write-routing-policy.md index eb93622..100987b 100644 --- a/docs/write-routing-policy.md +++ b/docs/write-routing-policy.md @@ -123,8 +123,10 @@ index state between calls. It may therefore coexist for reads; mutating tools refuse while another process owns the lease and reopen writable storage after that owner exits. - Read-only MCP HTTP may coexist with the writer. -- Read-only `sqlite_exact` clients use a `mode=ro`, `query_only` connection - and skip schema, WAL, FTS, migration, and metadata initialization. +- Read-only `sqlite_exact` clients use an immutable connection for a clean + checkpointed database, or `mode=ro` when an active writer's complete WAL + sidecar pair must remain visible. Both paths enable `query_only` and skip + schema, WAL, FTS, migration, and metadata initialization. - Direct CLI and hook writes must not run beside a writable daemon or MCP HTTP owner. Route them through the daemon with `require` when the daemon owns the palace. @@ -134,8 +136,9 @@ index state between calls. `MEMPALACE_MCP_ALLOW_PEER_WRITER` cannot bypass this protection for local file-backed or unknown plugin backends. It is retained only for explicitly -remote service backends (`qdrant` and `pgvector`) that coordinate concurrent -clients themselves. +remote service backends (`qdrant`, `pgvector`, and Milvus server/Zilliz Cloud) +that coordinate concurrent clients themselves. Milvus Lite remains protected +as local file-backed storage. Do not delete or unlink a live palace lock to recover ownership. Stop the owning process cleanly; the operating system releases its lock automatically. diff --git a/mempalace/backends/milvus.py b/mempalace/backends/milvus.py index 5dbc837..642a4ea 100644 --- a/mempalace/backends/milvus.py +++ b/mempalace/backends/milvus.py @@ -76,8 +76,11 @@ def _utcnow() -> str: return datetime.now(timezone.utc).isoformat() -def _is_server_uri(uri: str) -> bool: - normalized = uri.lower() +def milvus_uri_is_server(uri: Optional[str]) -> bool: + """Return whether ``uri`` targets service-managed Milvus storage.""" + if not uri: + return False + normalized = uri.strip().lower() return normalized.startswith(("http://", "https://", "tcp://", "grpc://")) @@ -358,7 +361,7 @@ class MilvusCollection(BaseCollection): if score is None: return 1.0 value = float(score) - if self._config.uri and _is_server_uri(self._config.uri): + if milvus_uri_is_server(self._config.uri): return 1.0 - max(-1.0, min(1.0, value)) return value @@ -976,7 +979,7 @@ class MilvusBackend(BaseBackend): collection_name: str, config: _MilvusConfig, ) -> str: - if config.uri and not _is_server_uri(config.uri) and not config.namespace: + if config.uri and not milvus_uri_is_server(config.uri) and not config.namespace: return _slug(collection_name) prefix = self._remote_collection_prefix(palace=palace, config=config) return f"{prefix}_{_slug(collection_name)}" @@ -1059,7 +1062,7 @@ class MilvusBackend(BaseBackend): client = self._clients.get(config) if client is not None: return client - if config.uri and not _is_server_uri(config.uri): + if config.uri and not milvus_uri_is_server(config.uri): parent = os.path.dirname(os.path.abspath(config.uri)) or "." os.makedirs(parent, exist_ok=True) try: @@ -1089,7 +1092,7 @@ class MilvusBackend(BaseBackend): pass marker_path = self._marker_path(palace.local_path) local_db_exists = bool( - config.uri and not _is_server_uri(config.uri) and os.path.exists(config.uri) + config.uri and not milvus_uri_is_server(config.uri) and os.path.exists(config.uri) ) if os.path.isfile(marker_path): self._validate_marker_target(palace, config) diff --git a/mempalace/backends/sqlite_exact.py b/mempalace/backends/sqlite_exact.py index 02c2f85..ee53b64 100644 --- a/mempalace/backends/sqlite_exact.py +++ b/mempalace/backends/sqlite_exact.py @@ -22,6 +22,7 @@ import numpy as np from .base import ( BackendClosedError, + BackendError, BaseBackend, BaseCollection, CollectionNotInitializedError, @@ -884,6 +885,31 @@ class SQLiteExactBackend(BaseBackend): def _db_path(palace_path: str) -> str: return os.path.join(palace_path, _DB_FILENAME) + @staticmethod + def _connect_read_only(db_path: str) -> sqlite3.Connection: + """Open without creating WAL files while preserving an active WAL.""" + wal_exists = os.path.isfile(f"{db_path}-wal") + shm_exists = os.path.isfile(f"{db_path}-shm") + if wal_exists != shm_exists: + raise BackendError( + "sqlite_exact read-only open found an incomplete WAL sidecar set; " + "open the palace after its writer exits cleanly or restore both " + "the -wal and -shm files" + ) + + db_uri = Path(db_path).resolve().as_uri() + if wal_exists: + # An active writer's uncheckpointed rows live in the WAL. With both + # sidecars already present, mode=ro can read them without creating + # filesystem state, including on a read-only mount. + db_uri = f"{db_uri}?mode=ro" + else: + # A clean WAL-mode database would otherwise make SQLite create new + # -wal/-shm files while connecting. Immutable mode is safe here + # because there is no WAL whose contents could be hidden. + db_uri = f"{db_uri}?mode=ro&immutable=1" + return sqlite3.connect(db_uri, uri=True, check_same_thread=False) + def _connect(self, palace_path: str, create: bool, *, read_only: bool = False): if self._closed: raise BackendClosedError("SQLiteExactBackend has been closed") @@ -913,12 +939,7 @@ class SQLiteExactBackend(BaseBackend): if cached is not None and not cached.closed: return cached if read_only: - db_uri = f"{Path(db_path).resolve().as_uri()}?mode=ro" - conn = sqlite3.connect( - db_uri, - uri=True, - check_same_thread=False, - ) + conn = self._connect_read_only(db_path) else: conn = sqlite3.connect(db_path, check_same_thread=False) try: diff --git a/mempalace/palace.py b/mempalace/palace.py index de874ba..fb03a82 100644 --- a/mempalace/palace.py +++ b/mempalace/palace.py @@ -355,7 +355,15 @@ def backend_requires_single_writer(backend_name: str) -> bool: plugin backends are treated conservatively. Only backends whose storage service is explicitly responsible for cross-process concurrency opt out. """ - return backend_name.strip().lower() not in _MULTI_PROCESS_WRITER_BACKENDS + normalized = backend_name.strip().lower() + if normalized == "milvus": + # Only embedded Milvus Lite is local single-writer storage. A remote + # Milvus server or Zilliz Cloud coordinates concurrent clients itself. + from .backends.milvus import milvus_uri_is_server + from .config import MempalaceConfig + + return not milvus_uri_is_server(MempalaceConfig().milvus_uri) + return normalized not in _MULTI_PROCESS_WRITER_BACKENDS def get_backend_for_palace(palace_path: str, explicit: Optional[str] = None): diff --git a/tests/test_palace.py b/tests/test_palace.py index 426924a..0580fe8 100644 --- a/tests/test_palace.py +++ b/tests/test_palace.py @@ -5,7 +5,36 @@ import chromadb from _chroma_palace_helper import make_minimal_chroma_sqlite from mempalace.backends import CollectionNotInitializedError, PalaceNotFoundError -from mempalace.palace import _open_collection_or_explain, get_collection +from mempalace.palace import ( + _open_collection_or_explain, + backend_requires_single_writer, + get_collection, +) + + +def test_backend_writer_ownership_distinguishes_milvus_lite_from_server(tmp_path, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + + monkeypatch.delenv("MEMPALACE_MILVUS_URI", raising=False) + assert backend_requires_single_writer("milvus") is True + + monkeypatch.setenv("MEMPALACE_MILVUS_URI", str(tmp_path / "milvus.db")) + assert backend_requires_single_writer("milvus") is True + + for uri in ( + "https://zilliz.example", + "http://milvus.example:19530", + "tcp://milvus.example:19530", + "grpc://milvus.example:19530", + ): + monkeypatch.setenv("MEMPALACE_MILVUS_URI", uri) + assert backend_requires_single_writer("milvus") is False + + +def test_backend_writer_ownership_remains_conservative_for_unknown_backend(): + assert backend_requires_single_writer("plugin_backend") is True + assert backend_requires_single_writer("qdrant") is False + assert backend_requires_single_writer("pgvector") is False def _capture(): diff --git a/tests/test_sqlite_exact_backend.py b/tests/test_sqlite_exact_backend.py index c73a9fd..3bc4f7b 100644 --- a/tests/test_sqlite_exact_backend.py +++ b/tests/test_sqlite_exact_backend.py @@ -402,23 +402,84 @@ def test_sqlite_exact_read_only_open_skips_schema_init_and_refuses_writes(tmp_pa db_path = tmp_path / "sqlite_exact.sqlite3" before = db_path.read_bytes() - read_only = backend.get_collection( - palace=palace, - collection_name="mempalace_drawers", - create=False, - options={"read_only": True}, - ) + assert not (tmp_path / "sqlite_exact.sqlite3-wal").exists() + assert not (tmp_path / "sqlite_exact.sqlite3-shm").exists() + db_path.chmod(0o400) + tmp_path.chmod(0o500) + try: + read_only = backend.get_collection( + palace=palace, + collection_name="mempalace_drawers", + create=False, + options={"read_only": True}, + ) - assert read_only.count() == 1 - assert read_only._handle.read_only is True - assert read_only._handle.conn.execute("PRAGMA query_only").fetchone()[0] == 1 - with pytest.raises(sqlite3.OperationalError): - read_only.add(ids=["b"], documents=["blocked"], metadatas=[{}], embeddings=[[1, 0]]) - - backend.close_palace(palace) + assert read_only.count() == 1 + assert read_only._handle.read_only is True + assert read_only._handle.conn.execute("PRAGMA query_only").fetchone()[0] == 1 + with pytest.raises(sqlite3.OperationalError): + read_only.add( + ids=["b"], + documents=["blocked"], + metadatas=[{}], + embeddings=[[1, 0]], + ) + assert not (tmp_path / "sqlite_exact.sqlite3-wal").exists() + assert not (tmp_path / "sqlite_exact.sqlite3-shm").exists() + finally: + tmp_path.chmod(0o700) + db_path.chmod(0o600) + backend.close_palace(palace) assert db_path.read_bytes() == before +def test_sqlite_exact_read_only_open_sees_active_writer_wal(tmp_path): + writer_backend, writer = _collection(tmp_path) + palace = PalaceRef(id=str(tmp_path), local_path=str(tmp_path)) + writer.add(ids=["wal-row"], documents=["uncheckpointed"], metadatas=[{}], embeddings=[[1, 0]]) + + db_path = tmp_path / "sqlite_exact.sqlite3" + wal_path = tmp_path / "sqlite_exact.sqlite3-wal" + shm_path = tmp_path / "sqlite_exact.sqlite3-shm" + assert wal_path.is_file() + assert shm_path.is_file() + before = {path: path.read_bytes() for path in (db_path, wal_path, shm_path)} + for path in before: + path.chmod(0o400) + tmp_path.chmod(0o500) + try: + reader_code = """ +import sys +from mempalace.backends import PalaceRef +from mempalace.backends.sqlite_exact import SQLiteExactBackend + +backend = SQLiteExactBackend() +palace = PalaceRef(id=sys.argv[1], local_path=sys.argv[1]) +reader = backend.get_collection( + palace=palace, + collection_name="mempalace_drawers", + create=False, + options={"read_only": True}, +) +print(reader.get(ids=["wal-row"]).documents[0]) +backend.close_palace(palace) +""" + result = subprocess.run( + [sys.executable, "-c", reader_code, str(tmp_path)], + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr + assert result.stdout.strip() == "uncheckpointed" + assert {path: path.read_bytes() for path in before} == before + finally: + tmp_path.chmod(0o700) + for path in before: + path.chmod(0o600) + writer_backend.close_palace(palace) + + def test_sqlite_exact_direct_write_contends_with_palace_owner(tmp_path, monkeypatch): from mempalace.palace import MineAlreadyRunning