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

Narek/release0.0.12 #265

Merged
merged 27 commits into from
Jan 24, 2024
Merged

Narek/release0.0.12 #265

merged 27 commits into from
Jan 24, 2024

Conversation

Ngalstyan4
Copy link
Contributor

No description provided.

Copy link

github-actions bot commented Jan 23, 2024

Benchmarks

metric old new pct change
recall (after create) 0.992 0.992 -
recall (after insert) 0.992 0.992 -
select bulk tps 50.904 50.677 -0.45%
select bulk latency (ms) 624.255 621.257 -0.48%
select bulk latency (stddev ms) 17.538 33.855 +93.04%
create latency (ms) 1274.159 1264.609 -0.75%
insert bulk tps 13.797 12.775 -7.41%
insert bulk latency (ms) 72.465 78.265 +8.00%
insert bulk latency (stddev ms) 4.291 5.132 +19.60%
disk usage (bytes) 6348800.000 6348800.000 -

Copy link

codecov bot commented Jan 23, 2024

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (7b66765) 81.72% compared to head (edd7b9f) 77.68%.

❗ Current head edd7b9f differs from pull request most recent head 0b83b5c. Consider uploading reports for the commit 0b83b5c to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #265      +/-   ##
==========================================
- Coverage   81.72%   77.68%   -4.05%     
==========================================
  Files          17       23       +6     
  Lines        1538     1963     +425     
  Branches      346      492     +146     
==========================================
+ Hits         1257     1525     +268     
- Misses        126      226     +100     
- Partials      155      212      +57     
Files Coverage Δ
src/hnsw/options.c 81.53% <100.00%> (+2.75%) ⬆️
src/hnsw/insert.c 79.41% <0.00%> (ø)
src/hnsw/build.c 80.81% <0.00%> (-0.61%) ⬇️
src/hnsw/utils.c 61.76% <0.00%> (-1.88%) ⬇️

... and 6 files with indirect coverage changes

Copy link
Collaborator

@var77 var77 left a comment

Choose a reason for hiding this comment

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

Looks good!

CMakeLists.txt Outdated
@@ -311,7 +333,7 @@ if (CLANG_FORMAT)

string(REGEX MATCH "[0-9]+\\.[0-9]+\\.[0-9]+" CLANG_FORMAT_VERSION "${CLANG_FORMAT_VERSION}")

if(CLANG_FORMAT_VERSION VERSION_LESS 14)
if(CLANG_FORMAT_VERSION VERSION_LESS 13)
message(WARNING "clang-format version ${CLANG_FORMAT_VERSION} found, need at least 14")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we change the version number in error message as well?

@@ -177,6 +177,17 @@ bool VersionsMatch()
version_checked = true;

SPI_finish();

if(!versions_match) {
elog(WARNING,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need the elog to be moved here? As now we will have duplicate logs for build and insert (as they're changed to WARNING) right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is logged once, as otherwise versions_match is cached.
it is good to have here for debugging, since it actually prints the version names and tells something about what the mismatch is

@Ngalstyan4 Ngalstyan4 merged commit 02c653f into main Jan 24, 2024
31 checks passed
@Ngalstyan4 Ngalstyan4 deleted the narek/release0.0.12 branch January 24, 2024 07:27
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.

None yet

2 participants