You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "hanahmily (via GitHub)" <gi...@apache.org> on 2026/08/25 12:48:12 UTC
[PR] feat(index): read the series index document count natively instead of through Bluge [skywalking-banyandb]
hanahmily opened a new pull request, #1308:
URL: https://github.com/apache/skywalking-banyandb/pull/1308
# Read the series index document count natively
Closes #14008. First slice of the native inverted index workstream (#14002).
## Why
`inverted.ReadOnlyDocCount` answers a small question — how many documents does
this closed series index hold? — and today it answers it by standing up a whole
Bluge reader: loading the segment plugin, resolving a snapshot generation,
mmapping every referenced segment file and building an index snapshot, all to
read one number that is already sitting in a 60-byte footer.
That is the cheapest place in the codebase to start replacing Bluge, and it is
also where the cost of not replacing it is most visible. The callers are all
inspection paths — closed-segment stats, snapshot verification, backup
round-trip checks, migration counts — and every one of them runs against
directories a live writer may still own. Opening a Bluge reader there means
taking shared locks on files the writer holds, falling back through generations
when those locks are unavailable, and mapping hundreds of kilobytes to read
eight bytes.
This change replaces the implementation and keeps the signature. Nothing in the
directory format changes, no index bytes are written, and rollback is a revert
of one function.
## What it does
`ReadOnlyDocCount` now delegates to a new internal package that reads the
directory directly. It enumerates the snapshot manifests, takes the newest
whose bytes decode, and sums each segment record's footer document count minus
the cardinality of that segment's logical deletion bitmap — which is exactly
the definition of the live count it replaces.
```mermaid
flowchart TD
caller["segment.SeriesIndexStats\nsnapshot verification\nbackup round-trip\nmigration counts"]
seam["inverted.ReadOnlyDocCount(path)"]
caller --> seam
seam --> native["icev3.LiveDocCount(dir)"]
subgraph reader["native reader (internal, no lock taken)"]
direction TB
enum["enumerate *.snp\nnewest generation first"]
parse["decode snapshot v3 manifest\nversion, segment records, deletion bitmaps"]
footer["read the 60-byte ICE v3 footer\nof each referenced segment"]
bounds["validate root offsets, versions\nand declared section lengths"]
sum["live = numDocs - deleted"]
enum --> parse --> footer --> bounds --> sum
end
native --> enum
sum --> ok(["count, nil"])
bounds -.->|"structural rule violated"| corrupt(["0, ErrCorruptSegment"])
parse -.->|"generation does not decode"| enum
enum -.->|"no generation decodes"| empty(["0, err — empty or unflushed"])
```
Three properties of that flow are worth calling out, because they are what the
old reader gave us for free and what the tests now pin explicitly.
**It never takes the directory lock, and never writes.** The whole point of this
seam is inspecting an index without reopening its writable form. The reader
opens files read-only and creates nothing, so a cold segment stays cold and a
live one is undisturbed.
**A torn generation degrades instead of failing.** Manifests are published in
place rather than by atomic rename, so a crash — or simply a read that lands
mid-publish — can leave a newest generation whose bytes stop half way through a
segment record. The reader steps back to the newest generation that does decode.
Only when none of them decode does the call report failure.
**"Empty" and "damaged" stay distinguishable.** Callers already treat a zero
count as an unflushed index and carry on; a directory that has no committed
generation yet must not be reported the same way as one whose bytes are wrong.
Structural violations — a segment shorter than its footer, a root offset outside
the file, an unsupported segment version, a declared section larger than the
file can hold — return a dedicated sentinel, and a caller can match on it.
The reserved CRC32 field at the tail of both file kinds is preserved in place
and otherwise ignored, matching the existing loader, which parses it but has
never enforced it.
## Compatibility
The format is untouched: no message changed, nothing new on the wire, no index
bytes written, no migration. The risk here is one-directional — the new reader
must agree with the old one about directories the old one wrote — so that is
what the tests cover.
A corpus generated by the pinned legacy writer is checked in alongside a
provenance manifest recording the writer version, the regeneration command, the
logical documents it contains, the per-file hashes and the declared count. The
tests read those bytes, assert the count the specification declares, and assert
the directory is byte-identical afterwards. A generator command lives next to
the corpus so it can be refreshed deliberately rather than drifting. Beyond the
fixture, the existing closed-segment and snapshot suites already assert real
counts over production-written series indexes with multiple segments and
deletion masks, and they gate this change unchanged.
The end-to-end test does what the callers do: commits two documents with the
legacy writer, leaves it open holding the directory lock, reads the count
through the public seam, and checks the published files are untouched — then
reads the same directory again once the writer has let go and gets the same
answer.
## Rollback
Restore the previous body of `ReadOnlyDocCount`. There is no drain, no replay,
no conversion and no file change, because this slice emits no index bytes.
Relates to apache/skywalking#14008.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(index): read the series index document count natively instead of through Bluge [skywalking-banyandb]
Posted by "hanahmily (via GitHub)" <gi...@apache.org>.
hanahmily closed pull request #1308: feat(index): read the series index document count natively instead of through Bluge
URL: https://github.com/apache/skywalking-banyandb/pull/1308
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(index): read the series index document count natively instead of through Bluge [skywalking-banyandb]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1308:
URL: https://github.com/apache/skywalking-banyandb/pull/1308#issuecomment-5410672181
## [Codecov](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1308?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 59.88%. Comparing base ([`3530dd9`](https://app.codecov.io/gh/apache/skywalking-banyandb/commit/3530dd9b5cefaa4fef60eec91ecbfbdfcaf243c5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`4ad806d`](https://app.codecov.io/gh/apache/skywalking-banyandb/commit/4ad806dafd211592e5a4f8f89d6f793c51846527?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 404 commits behind head on main.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #1308 +/- ##
===========================================
+ Coverage 45.97% 59.88% +13.90%
===========================================
Files 328 35 -293
Lines 55505 6259 -49246
===========================================
- Hits 25520 3748 -21772
+ Misses 27909 2288 -25621
+ Partials 2076 223 -1853
```
| [Flag](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1308/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [fodc](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1308/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `59.88% <ø> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1308?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org