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

Shard API (Mark shard(s) as read only) #1860

Merged
merged 24 commits into from Mar 17, 2022
Merged

Conversation

parkerduckworth
Copy link
Member

Changes

Introduce shard API, with endpoints:

  • GET /v1/schema/{className}/shards
  • PUT /v1/schema/{className}/shards/{shardName}

These endpoints facilitate marking a shard as READONLY, which is useful in situations where the shard must be immutable; taking a disk snapshot for example.

Under the hood, if a shard is marked READONLY, segment compaction and memtable flushing are halted until the shard is reset to READY. In this case, Weaviate will emit logs indicating that a compaction would have occurred, but was skipped due to the current shard status.


Fixes #1839

@codecov
Copy link

codecov bot commented Mar 15, 2022

Codecov Report

Merging #1860 (98d86bf) into master (9935355) will increase coverage by 0.14%.
The diff coverage is 75.91%.

@@            Coverage Diff             @@
##           master    #1860      +/-   ##
==========================================
+ Coverage   66.08%   66.23%   +0.14%     
==========================================
  Files         411      413       +2     
  Lines       30931    31333     +402     
==========================================
+ Hits        20442    20753     +311     
- Misses       8691     8768      +77     
- Partials     1798     1812      +14     
Flag Coverage Δ
integration 67.16% <44.66%> (-0.15%) ⬇️
unittests 66.23% <75.91%> (+0.14%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
adapters/handlers/rest/handlers_schema.go 0.00% <0.00%> (ø)
adapters/repos/db/lsmkv/strategies.go 80.00% <0.00%> (ø)
adapters/repos/db/migrator.go 30.64% <0.00%> (-5.90%) ⬇️
adapters/repos/db/shard_geo_props.go 52.00% <0.00%> (-6.21%) ⬇️
adapters/repos/db/shard_write_batch_objects.go 74.62% <0.00%> (+0.38%) ⬆️
adapters/repos/db/shard_write_batch_references.go 54.77% <0.00%> (-0.71%) ⬇️
adapters/repos/db/shard_write_delete.go 36.58% <0.00%> (-1.88%) ⬇️
adapters/repos/db/shard_write_merge.go 50.54% <0.00%> (-1.14%) ⬇️
adapters/repos/db/vector/hnsw/search.go 70.48% <0.00%> (ø)
adapters/repos/db/index.go 69.62% <9.09%> (-1.51%) ⬇️
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b2e9e6...98d86bf. Read the comment docs.

@@ -45,6 +46,9 @@ type Bucket struct {
legacyMapSortingBeforeCompaction bool

stopFlushCycle chan struct{}

status storagestate.Status
statusLock *sync.Mutex
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason why statusLock is a pointer?

Copy link
Member Author

Choose a reason for hiding this comment

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

I took the example from the attached internal issue video, which declared this field as a pointer. I don't have a reason aside from that 🙂 can change to value if preferred

@@ -41,6 +42,9 @@ type SegmentGroup struct {
// for backward-compatibility with states where the disk state for maps was
// not guaranteed to be sorted yet
mapRequiresSorting bool

status storagestate.Status
statusLock *sync.Mutex
Copy link
Contributor

Choose a reason for hiding this comment

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

here also is there a reason why statusLock is a pointer?

Copy link
Member Author

Choose a reason for hiding this comment

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

See above

go test -count 1 -coverpkg=./adapters/repos/... -coverprofile=coverage-integration.txt -race -tags=$tags "$@" ./adapters/repos/...
go test -count 1 -race -tags=$tags "$@" ./usecases/classification/...
go test -v -count 1 -coverpkg=./adapters/repos/... -coverprofile=coverage-integration.txt -race -tags=$tags "$@" ./adapters/repos/...
go test -v -count 1 -race -tags=$tags "$@" ./usecases/classification/...
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should remove -v or better maybe introduce a param for example --verbose that would add this -v if needed

Copy link
Member Author

Choose a reason for hiding this comment

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

ok will do 👍

@parkerduckworth parkerduckworth merged commit a37cdf3 into master Mar 17, 2022
@parkerduckworth parkerduckworth deleted the gh-1839-read-only-shards branch August 11, 2022 16:06
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.

Mark shard(s) as read only
3 participants