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.

Sign up for GitHub

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

Jump to bottom

clp-s: Add archive_id to search results. #435

Merged
merged 21 commits into from
Jun 14, 2024
Merged

clp-s: Add archive_id to search results. #435

merged 21 commits into from
Jun 14, 2024

Conversation

haiqi96
Copy link
Contributor

@haiqi96 haiqi96 commented Jun 10, 2024 •
edited
Loading

References

Description

This PR adds archive_id to the search results of clp-s and a few refactorings:

  1. Let clp-s:ArchiveReader tracks archive_id and update the signature of open() function.
  2. replace std::string with std::string_view in clp-s' OutputHandler
  3. replace "should_output_timestamp" with "should_output_metadata"

Validation performed

  1. Compressed and searched mongod log with clp-s binary directly. Verified that results are output properly
  2. Ingested mongod log with clp-s package. Verified that both search.sh and webui can return the results properly
  3. Confirmed that messages in the results_cache have associated archive_id.

@haiqi96 haiqi96 marked this pull request as draft June 10, 2024 19:20
@haiqi96 haiqi96 marked this pull request as ready for review June 10, 2024 19:55
@haiqi96 haiqi96 changed the title Add archive_id to the search results of clp-s clp-s: Add archive_id to the search results of clp-s Jun 10, 2024
kirkrodrigues
kirkrodrigues requested changes Jun 13, 2024
View reviewed changes
components/core/src/clp_s/search/OutputHandler.hpp Show resolved Hide resolved
components/core/src/clp_s/ArchiveReader.cpp Show resolved Hide resolved
components/core/src/clp_s/clp-s.cpp Show resolved Hide resolved
components/core/src/clp_s/clp-s.cpp Outdated Show resolved Hide resolved
components/core/src/clp_s/search/OutputHandler.cpp Show resolved Hide resolved
components/core/src/clp_s/search/OutputHandler.cpp Outdated Show resolved Hide resolved
components/core/src/clp_s/search/OutputHandler.cpp Outdated Show resolved Hide resolved
components/core/src/clp_s/search/OutputHandler.hpp Outdated Show resolved Hide resolved
components/core/src/clp_s/search/OutputHandler.hpp Outdated Show resolved Hide resolved
kirkrodrigues
kirkrodrigues requested changes Jun 14, 2024
View reviewed changes
components/core/src/clp_s/search/Output.cpp Outdated Show resolved Hide resolved
components/core/src/clp_s/ArchiveReader.cpp Outdated Show resolved Hide resolved
@haiqi96 haiqi96 requested a review from kirkrodrigues June 14, 2024 01:49
kirkrodrigues
kirkrodrigues approved these changes Jun 14, 2024
View reviewed changes
Copy link
Member

@kirkrodrigues kirkrodrigues left a comment

Choose a reason for hiding this comment

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

For the PR title, how about:

clp-s: Add archive_id to search results.

@haiqi96 haiqi96 changed the title clp-s: Add archive_id to the search results of clp-s clp-s: Add archive_id to search results. Jun 14, 2024
@haiqi96 haiqi96 merged commit eedbd20 into y-scope:main Jun 14, 2024
11 checks passed
@haiqi96 haiqi96 deleted the CLPSArchiveID branch June 28, 2024 14:43
jackluo923 pushed a commit to jackluo923/clp that referenced this pull request Dec 4, 2024
@haiqi96
clp-s: Add archive_id to search results. (y-scope#435)
0af9f08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wraymo wraymo wraymo left review comments

@kirkrodrigues kirkrodrigues kirkrodrigues approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@haiqi96 @kirkrodrigues @wraymo