feat(connectors): ETag-based bypass for incremental S3 ingestion (#14628) (#14677)

### What problem does this PR solve?

S3-family connector syncs currently re-download every in-window object
just so we can compute `xxhash128(blob)` and compare against
`Document.content_hash`. Anything that bumps `LastModified` without
changing bytes (`aws s3 cp` touches, bucket re-encryption, etc.) pays
full bandwidth and re-parses files that didn't actually change. #14628
covers the broader incremental-ingestion redesign; this PR is the first
slice.

The fix is a pre-listing short-circuit. `BlobStorageConnector` (S3 / R2
/ GCS / OCI / S3-compat) now implements a new `FingerprintConnector`
interface: `list_keys()` paginates `list_objects_v2` and yields
`KeyRecord(key, fingerprint)` where `fingerprint = xxhash128(ETag)`. The
orchestrator joins those against the connector's existing `{doc_id:
content_hash}` map and only calls `get_value(key)` when the fingerprint
differs. Unchanged keys are skipped entirely — no `GetObject`, no
re-parse.

No DDL. xxhash128(ETag) is 32 hex chars and reuses the existing
`Document.content_hash` column per @yingfeng's suggestion; the connector
decides at listing time whether to populate it. Local uploads and
connectors that don't opt in fall through to the existing post-download
`xxhash128(blob)` path with no behavior change.

This is PR-1 of a 4-PR series — full design lives on #14628. Subsequent
PRs extend tier 1 to local FS / WebDAV / Dropbox / Seafile / RDBMS
(PR-2), wire up tier 2 cursor connectors with `SyncLogs.next_checkpoint`
(PR-3), and unify deletion via `KeyRecord(deleted=True)` reconciliation
(PR-4). Holding those back keeps this PR additive and reviewable on its
own.

#### Files touched

- `common/data_source/models.py` — new `KeyRecord`; optional
`fingerprint` on `Document`
- `common/data_source/interfaces.py` — `IncrementalCapability` enum,
`FingerprintConnector` ABC
- `common/data_source/blob_connector.py` — `BlobStorageConnector`
implements `FingerprintConnector`; per-object download factored into
`_build_document_from_obj()` so `_yield_blob_objects`, `list_keys`,
`get_value` all share it
- `rag/svr/sync_data_source.py` —
`_BlobLikeBase._fingerprint_filtered_generator` does the bypass loop;
`_run_task_logic` plumbs `doc.fingerprint` into the upload dict
- `api/db/services/document_service.py` —
`list_id_content_hash_map_by_kb_and_source_type()` helper
- `api/db/services/connector_service.py` + `file_service.py` —
fingerprint flows through `duplicate_and_parse → upload_document` and
lands in `content_hash`
- `test/unit_test/common/test_blob_connector_fingerprint.py` — 14 tests
covering ETag normalization (single-part, multipart, quoted, empty),
`list_keys()` not calling `GetObject`, `get_value()` materializing with
fingerprint, deterministic/stable fingerprints, and the bypass loop
asserting `GetObject` is *not* called on a match

#### Worth flagging for review

Old `_BlobLikeBase._generate` called `poll_source(start, now)` with a
`LastModified` window when `poll_range_start` was set. New code uses
`_fingerprint_filtered_generator` (full bucket listing + fingerprint
compare) outside of explicit `reindex=1`. Strictly better for
unchanged-bucket cases since it skips `GetObject`, but it does mean
every sync now does a full `list_objects_v2` paginate. Should still be
cheap for most buckets — flagging in case anyone has a very large bucket
where the time-window filter was meaningful.

On migration: existing rows have `content_hash = xxhash128(blob)` from
the old code. The first sync after this lands sees ETag-derived
fingerprints that don't match, re-fetches every object once, and writes
the new fingerprint. From the second sync onward the bypass works as
expected. "Slow day one, fast every day after." A `fingerprint_backfill:
trust` opt-out is sketched in the design doc but not in this PR.

#### Test plan

- [x] `uv run ruff check` — clean on all 8 touched files
- [x] `uv run pytest
test/unit_test/common/test_blob_connector_fingerprint.py -v` — 14 passed
- [x] Broader unit-test suite — no regressions in anything I touched
- [ ] Manual smoke against a real S3 bucket — configure a connector, run
sync twice, expect the second sync to log `bypassed=N, fetched=0` and no
`GetObject` calls in CloudTrail / bucket access logs
- [ ] Manual smoke with `reindex=1` — confirm the full re-download path
still works

### Type of change

- [x] New Feature (non-breaking change which adds functionality)

---------

Co-authored-by: Yingfeng <yingfeng.zhang@gmail.com>
This commit is contained in:
Hunnyboy1217
2026-05-09 05:03:56 -07:00
committed by GitHub
parent 7931b693dc
commit 782084780e
8 changed files with 658 additions and 53 deletions

View File

@@ -213,6 +213,8 @@ class SyncBase:
}
if doc.metadata:
d["metadata"] = doc.metadata
if getattr(doc, "fingerprint", None):
d["fingerprint"] = doc.fingerprint
docs.append(d)
try:
@@ -301,6 +303,81 @@ class SyncBase:
class _BlobLikeBase(SyncBase):
DEFAULT_BUCKET_TYPE: str = "s3"
def _fingerprint_filtered_generator(self, task: dict):
"""Generator that uses list_keys() + get_value() to skip unchanged objects.
Pre-loads {doc_id: content_hash} for the connector's existing docs in
this KB, iterates the bucket via list_keys(), and only materializes a
Document (one GetObject call) when the listing fingerprint differs from
the persisted content_hash. Unchanged objects are skipped entirely --
no download, no re-parse.
Per-key fetch failures are counted and surfaced via SyncLogsService so
a partially failing sync (e.g. throttling, IAM regression mid-run)
doesn't silently report DONE while half the bucket is unreachable.
Connectors yielding KeyRecord(deleted=True) are skipped here -- actual
deletion reconciliation lives in the unified delete pass (PR-4).
"""
source_type = f"{self.SOURCE_NAME}/{task['connector_id']}"
existing_fingerprints = DocumentService.list_id_content_hash_map_by_kb_and_source_type(
task["kb_id"], source_type,
)
bypass_count = 0
fetch_count = 0
fail_count = 0
batch = []
for key_record in self.connector.list_keys():
if key_record.deleted:
continue
doc_id = hash128(key_record.key)
stored = existing_fingerprints.get(doc_id, "")
if key_record.fingerprint and stored and key_record.fingerprint == stored:
bypass_count += 1
continue
try:
doc = self.connector.get_value(key_record.key)
except Exception as ex:
fail_count += 1
logging.exception(
"Failed to fetch %s from %s: %s",
key_record.key,
self.SOURCE_NAME,
ex,
)
continue
fetch_count += 1
batch.append(doc)
if len(batch) >= self.connector.batch_size:
yield batch
batch = []
if batch:
yield batch
log_msg = (
"[%s] fingerprint sync: %d bypassed, %d fetched, %d failed "
"(connector_id=%s, kb_id=%s)"
)
log_args = (
self.SOURCE_NAME,
bypass_count,
fetch_count,
fail_count,
task["connector_id"],
task["kb_id"],
)
# Use WARNING when any fetch failed so partial-bucket regressions
# (auth, throttling, IAM drift) surface without diving into the
# per-exception traces above.
if fail_count:
logging.warning(log_msg, *log_args)
else:
logging.info(log_msg, *log_args)
async def _generate(self, task: dict):
bucket_type = self.conf.get("bucket_type", self.DEFAULT_BUCKET_TYPE)
@@ -313,14 +390,13 @@ class _BlobLikeBase(SyncBase):
self.connector.load_credentials(self.conf["credentials"])
file_list = None
document_batch_generator = (
self.connector.load_from_state()
if task["reindex"] == "1" or not task["poll_range_start"]
else self.connector.poll_source(
task["poll_range_start"].timestamp(),
datetime.now(timezone.utc).timestamp(),
)
)
# Fingerprint-bypass path: skip GetObject for unchanged ETags. Disabled
# on full reindex (we want to re-fetch everything in that case).
use_fingerprint_path = task["reindex"] != "1"
if use_fingerprint_path:
document_batch_generator = self._fingerprint_filtered_generator(task)
else:
document_batch_generator = self.connector.load_from_state()
if (
task["reindex"] != "1"
@@ -332,9 +408,9 @@ class _BlobLikeBase(SyncBase):
file_list.extend(slim_batch)
_begin_info = (
"totally"
if task["reindex"] == "1" or not task["poll_range_start"]
else "from {}".format(task["poll_range_start"])
"fingerprint-bypass"
if use_fingerprint_path
else "full reindex"
)
logging.info(