Connector checkpointing #3876

Weves · 2025-02-02T00:05:20Z

Description

https://linear.app/danswer/issue/DAN-1400/connector-checkpointing-continue-on-failure

How Has This Been Tested?

New integration tests
Testing Slack connector locally with induced failures + verifying that it indexes the same # of docs as the old connector
Tested that Google Drive connector indexes the same # of docs

Backporting (check the box to trigger backport action)

Note: You have to check that the action passes, otherwise resolve the conflicts manually and tag the patches.

This PR should be backported (make sure to check that the backport attempt succeeds)
[Optional] Override Linear Check

vercel · 2025-02-02T00:05:25Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
internal-search	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 2, 2025 11:04pm

Weves · 2025-02-04T02:24:40Z

backend/onyx/connectors/slack/connector.py

+                            yield (doc, checkpoint, None)
+
+                        if thread_ts:
+                            seen_thread_ts.add(thread_ts)


maybe actually still wrong?

Weves · 2025-02-04T02:48:17Z

backend/onyx/connectors/interfaces.py

+    @abc.abstractmethod
+    def load_from_checkpoint(
+        self,
+        start: SecondsSinceUnixEpoch,


Add comment

Weves · 2025-02-04T02:49:40Z

backend/onyx/server/documents/indexing.py


 router = APIRouter(prefix="/manage")


-@router.get("/admin/indexing-errors/{index_attempt_id}")
+@router.get("/admin/index-attempt/{index_attempt_id}/errors")


TODO: delete this one

Weves · 2025-02-04T02:52:45Z

backend/onyx/background/indexing/checkpointing_utils.py

+    window_end: datetime,
+) -> ConnectorCheckpoint:
+    """Get the latest valid checkpoint for a given connector credential pair"""
+    checkpoint_candidates = get_recent_completed_attempts_for_cc_pair(


add a limit to the number of tries for the same checkpoint

rkuo-danswer · 2025-02-03T08:40:44Z

backend/alembic/versions/b7a7eee5aa15_add_checkpointing_failure_handling.py

+        sa.Column("checkpoint_pointer", sa.String(), nullable=True),
+    )
+    op.add_column(
+        "index_attempt",


this seems extremely specific to the concept of a poll start and end time. Are we sure checkpoints will always be time based?

rkuo-danswer · 2025-02-03T08:43:19Z

backend/onyx/background/celery/tasks/indexing/tasks.py

+@shared_task(
+    name=OnyxCeleryTask.CHECK_FOR_CHECKPOINT_CLEANUP,
+    soft_time_limit=300,
+    bind=True,


is the bind used here? maybe we can leave it out?

rkuo-danswer · 2025-02-03T23:16:38Z

backend/onyx/server/documents/cc_pair.py

+    _: User = Depends(current_curator_or_admin_user),
+    db_session: Session = Depends(get_session),
+) -> PaginatedReturn[IndexAttemptErrorPydantic]:
+    total_count = count_index_attempt_errors_for_cc_pair(


docstring for endpoint?

rkuo-danswer · 2025-02-03T23:17:04Z

backend/onyx/server/documents/indexing.py


 router = APIRouter(prefix="/manage")


-@router.get("/admin/indexing-errors/{index_attempt_id}")
+@router.get("/admin/index-attempt/{index_attempt_id}/errors")


rkuo-danswer · 2025-02-03T23:19:21Z

backend/onyx/indexing/indexing_pipeline.py

@@ -519,6 +510,7 @@ def index_doc_batch(
        new_docs=len([r for r in insertion_records if r.already_existed is False]),
        total_docs=len(filtered_documents),
        total_chunks=len(access_aware_chunks),
+        failures=vector_db_write_failures + embedding_falures,


embedding_falures -> failures

rkuo-danswer · 2025-02-03T23:40:07Z

backend/onyx/connectors/models.py

+        if (failed_document is None and failed_entity is None) or (
+            failed_document is not None and failed_entity is not None
+        ):
+            raise ValueError(


Should this be a Union type then?

rkuo-danswer · 2025-02-03T23:41:21Z

backend/onyx/connectors/models.py

-            id=doc.id, semantic_id=doc.semantic_identifier, section_link=section_link
-        )
+
+class ConnectorCheckpoint(BaseModel):


maybe put in a check somewhere so that at least we can't write arbitrarily large checkpoints

rkuo-danswer · 2025-02-03T23:51:41Z

backend/onyx/background/indexing/run_indexing.py

+    1. We have more than 3 failures AND
+    2. Failures account for more than 10% of processed documents
+    """
+    if total_failures > 3 and total_failures / (document_count or 1) > 0.1:


could reduce nesting here for readability/flow

rkuo-danswer · 2025-02-04T00:08:44Z

backend/onyx/background/celery/tasks/beat_schedule.py

@@ -35,6 +35,19 @@
            "expires": BEAT_EXPIRES_DEFAULT,
        },
    },
+    {


should put after the cloud generator comment

rkuo-danswer · 2025-02-04T00:18:54Z

backend/onyx/background/indexing/run_indexing.py

+    except Exception as e:
+        logger.exception(
+            "Connector run exceptioned after elapsed time: "
+            f"{time.time() - start_time} seconds"


time.monotonic probably better here?

vercel bot deployed to Preview February 2, 2025 00:12 View deployment

vercel bot deployed to Preview February 2, 2025 01:42 View deployment

Weves added 13 commits February 1, 2025 23:10

wip checkpointing/continue on failure

1877cfb

more stuff for checkpointing

7523a6a

Basic implementation

2bac268

FE stuff

c02edd8

More checkpointing/failure handling

168bef4

rebase

eea3a34

rebase

541d104

initial scaffolding for IT

fcc4817

IT to test checkpointing

0ee2b84

Cleanup

cd7e3f1

cleanup

827c9f7

Fix it

3c5bc68

Rebase

f69739e

Weves force-pushed the connector-checkpointing branch from b0e50cf to f69739e Compare February 2, 2025 19:50

vercel bot deployed to Preview February 2, 2025 19:55 View deployment

Weves added 2 commits February 2, 2025 12:23

Add todo

ee74494

Fix actions IT

5929d5b

vercel bot deployed to Preview February 2, 2025 20:31 View deployment

Weves added 3 commits February 2, 2025 12:51

Test more

8843351

Pagination + fixes + cleanup

7869a5c

Fix IT networking

3944a61

Weves marked this pull request as ready for review February 2, 2025 22:01

vercel bot deployed to Preview February 2, 2025 22:03 View deployment

fix it

320b62d

vercel bot deployed to Preview February 2, 2025 23:04 View deployment

Weves commented Feb 4, 2025

View reviewed changes

rkuo-danswer reviewed Feb 4, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Connector checkpointing #3876

Connector checkpointing #3876

Weves commented Feb 2, 2025 •

edited

Loading

vercel bot commented Feb 2, 2025 •

edited

Loading

Weves Feb 4, 2025

Weves Feb 4, 2025

Weves Feb 4, 2025

Weves Feb 4, 2025

rkuo-danswer Feb 3, 2025

rkuo-danswer Feb 3, 2025

rkuo-danswer Feb 3, 2025

rkuo-danswer Feb 3, 2025

rkuo-danswer Feb 3, 2025

rkuo-danswer Feb 3, 2025

rkuo-danswer Feb 3, 2025

rkuo-danswer Feb 3, 2025

rkuo-danswer Feb 4, 2025

rkuo-danswer Feb 4, 2025

Connector checkpointing #3876

Are you sure you want to change the base?

Connector checkpointing #3876

Conversation

Weves commented Feb 2, 2025 • edited Loading

Description

How Has This Been Tested?

Backporting (check the box to trigger backport action)

vercel bot commented Feb 2, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Weves commented Feb 2, 2025 •

edited

Loading

vercel bot commented Feb 2, 2025 •

edited

Loading