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

Timing Logging Enrichment #242

Merged
merged 8 commits into from
Dec 22, 2022
Merged

Timing Logging Enrichment #242

merged 8 commits into from
Dec 22, 2022

Conversation

vicilliar
Copy link
Contributor

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    Observability feature.

  • What is the current behavior? (You can also link to an open issue here)
    Timing logs are not output at every step of Marqo add_documents and search. They are only returned to the client via processingTimeMS.

  • What is the new behavior (if this is a feature change)?
    Breaks down and adds explicit timing logs to relevant parts of the Marqo process. Replaces existing timing devices with timeit.default_timer, due to it being faster to instantiate than datetime.datetime.

The following parts now have explicit timing logs (by batch):

  1. add_documents - pre-processing (total time)
  2. add_documents - pre-processing (vectorisation time). Individual vectorisation time is sent as logger.debug
  3. add_documents - HTTP roundtrip (Marqo <-> Marqo-os)
  4. add_documents - Indexing time (from Marqo-os result)
  5. search - pre-processing (total time)
  6. search - HTTP roundtrip (Marqo <-> Marqo-os)
  7. search - post-processing (total time)
  8. search - reranking
  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    No.

  • Have unit tests been run against this PR? (Has there also been any additional testing?)
    Yes.

  • Related Python client changes (link commit/PR here)

  • Related documentation changes (link commit/PR here)

  • Other information:

  • Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes/features)
  • Docs have been added / updated (for bug fixes / features)

src/marqo/tensor_search/tensor_search.py Show resolved Hide resolved
src/marqo/tensor_search/tensor_search.py Outdated Show resolved Hide resolved
src/marqo/tensor_search/tensor_search.py Show resolved Hide resolved
added Marqo-os search time to both lexical and tensor search. added total search time log. updated descriptions of search to say whether multi or normal search is used.
Copy link
Collaborator

@pandu-k pandu-k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left comment

src/marqo/tensor_search/tensor_search.py Show resolved Hide resolved
@vicilliar vicilliar temporarily deployed to marqo-test-suite December 21, 2022 11:39 — with GitHub Actions Inactive
@vicilliar vicilliar temporarily deployed to marqo-test-suite December 21, 2022 18:03 — with GitHub Actions Inactive
@pandu-k pandu-k merged commit e2c9d81 into mainline Dec 22, 2022
@pandu-k pandu-k deleted the timing-logging branch December 29, 2022 03:25
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

Successfully merging this pull request may close these issues.

2 participants