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

[wip] Jsoncs3 investigate #3935

Closed
wants to merge 14 commits into from
Closed

[wip] Jsoncs3 investigate #3935

wants to merge 14 commits into from

Conversation

dragotin
Copy link
Contributor

@dragotin dragotin commented Jun 5, 2023 •
edited by dragonchaser
Loading

Bugfix: We use read locks when listing shares

We use read locks when listing shares

@dragotin dragotin force-pushed the jsoncs3_investigate branch from 59850b8 to 72d901e Compare June 5, 2023 12:30
@dragotin dragotin force-pushed the jsoncs3_investigate branch 2 times, most recently from 69afe9b to 7fb083c Compare June 5, 2023 15:22
dragonchaser
dragonchaser previously requested changes Jun 5, 2023
View reviewed changes
pkg/share/manager/jsoncs3/jsoncs3.go Outdated Show resolved Hide resolved
pkg/share/manager/jsoncs3/jsoncs3.go Outdated Show resolved Hide resolved
@dragonchaser dragonchaser dismissed their stale review June 5, 2023 15:40

co-author

@butonic butonic mentioned this pull request Jun 6, 2023
Draft
@dragonchaser dragonchaser force-pushed the jsoncs3_investigate branch from 7e0b68b to ba86321 Compare June 6, 2023 09:08
kobergj
kobergj requested changes Jun 6, 2023
View reviewed changes
Copy link
Contributor

@kobergj kobergj left a comment

Choose a reason for hiding this comment

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

Some open questions

pkg/share/manager/jsoncs3/jsoncs3.go Outdated Show resolved Hide resolved
pkg/share/manager/jsoncs3/jsoncs3.go Outdated Show resolved Hide resolved
pkg/share/manager/jsoncs3/jsoncs3.go Outdated Show resolved Hide resolved
pkg/share/manager/jsoncs3/jsoncs3.go Outdated Show resolved Hide resolved
pkg/share/manager/jsoncs3/jsoncs3.go Outdated Show resolved Hide resolved
pkg/share/manager/jsoncs3/jsoncs3.go Show resolved Hide resolved
pkg/share/manager/jsoncs3/jsoncs3.go Show resolved Hide resolved
pkg/share/manager/jsoncs3/jsoncs3.go Show resolved Hide resolved
@dragonchaser dragonchaser changed the title [full-ci] [wip] Jsoncs3 investigate [wip] Jsoncs3 investigate Jun 6, 2023
@kobergj
Copy link
Contributor

kobergj commented Jun 6, 2023

Testing ocis full-ci here: owncloud/ocis#6445

@dragonchaser dragonchaser force-pushed the jsoncs3_investigate branch 3 times, most recently from d8abf06 to a2480a5 Compare June 6, 2023 11:06
Klaas Freitag and others added 14 commits June 8, 2023 00:24
@butonic
Use RLock instead of Lock
78926a8
@butonic
Add Changelog
3102c7e
@butonic
More finegranular locking for getReceived
1dfcce1
@butonic
Add missing Unlocks before returns
a0a348a
@butonic
Do not write in read functions - postpone removal of expired shares.
1835b8a 
The reading functions just ignore the expired shares. They are removed
by an asynchronous method later from the file on CS3.
@butonic
Extract cache invalidation to extra function and call only on cache
4fdfcba 
update
@butonic
Make linter happy with Errorf
9d1aa07
@dragonchaser @butonic
Update pkg/share/manager/jsoncs3/jsoncs3.go
1dd737c
@dragonchaser @butonic
Update pkg/share/manager/jsoncs3/jsoncs3.go
5408006
@dragonchaser @butonic
move expiry event to invalidateCache func
470073a 
Signed-off-by: Christian Richter <[email protected]>
@dragonchaser @butonic
call invalidateCache on every cache Write
94fb66a 
Signed-off-by: Christian Richter <[email protected]>
@dragonchaser @butonic
cleanup
3a823f1 
Signed-off-by: Christian Richter <[email protected]>
@dragonchaser @butonic
remove obsolete event emit
37f14fb 
Signed-off-by: Christian Richter <[email protected]>
@butonic
fix panic when expiry is unset
fa09a8e 
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic butonic force-pushed the jsoncs3_investigate branch from fbc3ca4 to fa09a8e Compare June 7, 2023 22:24
@butonic
Copy link
Contributor

butonic commented Jun 7, 2023

fixed a panic:

goroutine 2906 [running]:                                                                                                                                                                      │
│ runtime/debug.Stack()                                                                                                                                                                          │
│     runtime/debug/stack.go:24 +0x65                                                                                                                                                            │
│ runtime/debug.PrintStack()                                                                                                                                                                     │
│     runtime/debug/stack.go:16 +0x19                                                                                                                                                            │
│ github.com/cs3org/reva/v2/internal/grpc/interceptors/recovery.recoveryFunc({0x44a27d8, 0xc001d7f9b0}, {0x3f60a80, 0x5cbad50})                                                                  │
│     github.com/cs3org/reva/[email protected]/internal/grpc/interceptors/recovery/recovery.go:48 +0x3a                                                                   │
│ github.com/grpc-ecosystem/go-grpc-middleware/recovery.recoverFrom({0x44a27d8?, 0xc001d7f9b0?}, {0x3f60a80?, 0x5cbad50?}, 0x0?)                                                                 │
│     github.com/grpc-ecosystem/[email protected]/recovery/interceptors.go:61 +0x36                                                                                                      │
│ github.com/grpc-ecosystem/go-grpc-middleware/recovery.UnaryServerInterceptor.func1.1()                                                                                                         │
│     github.com/grpc-ecosystem/[email protected]/recovery/interceptors.go:29 +0x7b                                                                                                      │
│ panic({0x3f60a80, 0x5cbad50})                                                                                                                                                                  │
│     runtime/panic.go:884 +0x213                                                                                                                                                                │
│ go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End.func1()                                                                                                                                │
│     go.opentelemetry.io/otel/[email protected]/trace/span.go:383 +0x2a                                                                                                                               │
│ go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End(0xc002305980, {0x0, 0x0, 0x20606e56a9bb65bf?})                                                                                         │
│     go.opentelemetry.io/otel/[email protected]/trace/span.go:421 +0xa29                                                                                                                              │
│ panic({0x3f60a80, 0x5cbad50})                                                                                                                                                                  │
│     runtime/panic.go:884 +0x213                                                                                                                                                                │
│ go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End.func1()                                                                                                                                │
│     go.opentelemetry.io/otel/[email protected]/trace/span.go:383 +0x2a                                                                                                                               │
│ go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End(0xc002305c80, {0x0, 0x0, 0x20606e56a9bb65bf?})                                                                                         │
│     go.opentelemetry.io/otel/[email protected]/trace/span.go:421 +0xa29                                                                                                                              │
│ panic({0x3f60a80, 0x5cbad50})                                                                                                                                                                  │
│     runtime/panic.go:884 +0x213                                                                                                                                                                │
│ go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End.func1()                                                                                                                                │
│     go.opentelemetry.io/otel/[email protected]/trace/span.go:383 +0x2a                                                                                                                               │
│ go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End(0xc002305e00, {0x0, 0x0, 0xc00152c360?})                                                                                               │
│     go.opentelemetry.io/otel/[email protected]/trace/span.go:421 +0xa29                                                                                                                              │
│ panic({0x3f60a80, 0x5cbad50})                                                                                                                                                                  │
│     runtime/panic.go:890 +0x263                                                                                                                                                                │
│ github.com/cs3org/reva/v2/pkg/share.IsExpired(0xc00152c360?)                                                                                                                                   │
│     github.com/cs3org/reva/[email protected]/pkg/share/share.go:245 +0x18                                                                                               │
│ github.com/cs3org/reva/v2/pkg/share/manager/jsoncs3.(*Manager).invalidateCache(0xc00152c360, {0x44a27d8, 0xc001d7fce0}, 0xc001d497a0)                                                          │
│     github.com/cs3org/reva/[email protected]/pkg/share/manager/jsoncs3/jsoncs3.go:726 +0x3c5                                                                            │
│ github.com/cs3org/reva/v2/pkg/share/manager/jsoncs3.(*Manager).Share(0xc00152c360, {0x44a27d8, 0xc001d7fcb0}, 0xc001d5b2c0, 0xc001d26e40)                                                      │
│     github.com/cs3org/reva/[email protected]/pkg/share/manager/jsoncs3/jsoncs3.go:296 +0x4be                                                                            │
│ github.com/cs3org/reva/v2/internal/grpc/services/usershareprovider.(*service).CreateShare(0xc000f79410, {0x44a27d8, 0xc001d7fcb0}, 0xc001d26d80) 
│     github.com/cs3org/reva/[email protected]/internal/grpc/services/usershareprovider/usershareprovider.go:151 +0x1c3                                                   │
│ github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1._CollaborationAPI_CreateShare_Handler.func1({0x44a27d8, 0xc001d7fcb0}, {0x42a2b20?, 0xc001d26d80})                              │
│     github.com/cs3org/[email protected]/cs3/sharing/collaboration/v1beta1/collaboration_api.pb.go:1411 +0x78                                                       │
│ go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc.UnaryServerInterceptor.func1({0x44a27d8, 0xc001d7fbc0}, {0x42a2b20, 0xc001d26d80}, 0xc001cbf2e0, 0xc0017abf98)     │
│     go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/[email protected]/interceptor.go:341 +0x437                                                                              │
│ github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func2.1({0x44a27d8?, 0xc001d7fbc0?}, {0x42a2b20?, 0xc001d26d80?})                                                                │
│     github.com/grpc-ecosystem/[email protected]/chain.go:48 +0x4f                                                                                                                      │
│ github.com/cs3org/reva/v2/internal/grpc/interceptors/eventsmiddleware.NewUnary.func1({0x44a27d8, 0xc001d7fb90}, {0x42a2b20?, 0xc001d26d80?}, 0x0?, 0xc001d26f40)                               │
│     github.com/cs3org/reva/[email protected]/internal/grpc/interceptors/eventsmiddleware/events.go:72 +0xe8                                                             │
│ github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func2.1({0x44a27d8?, 0xc001d7fb90?}, {0x42a2b20?, 0xc001d26d80?})                                                                │
│     github.com/grpc-ecosystem/[email protected]/chain.go:48 +0x4f                                                                                                                      │
│ github.com/cs3org/reva/v2/internal/grpc/interceptors/prometheus.interceptorFromConfig.func1({0x44a27d8?, 0xc001d7fb90?}, {0x42a2b20?, 0xc001d26d80?}, 0x0?, 0xc001f4cd60?)                     │
│     github.com/cs3org/reva/[email protected]/internal/grpc/interceptors/prometheus/prometheus.go:72 +0x43                                                               │
│ github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func2.1({0x44a27d8?, 0xc001d7fb90?}, {0x42a2b20?, 0xc001d26d80?})                                                                │
│     github.com/grpc-ecosystem/[email protected]/chain.go:48 +0x4f                                                                                                                      │
│ github.com/cs3org/reva/v2/internal/grpc/interceptors/auth.NewUnary.func2({0x44a27d8, 0xc001d7f9b0}, {0x42a2b20, 0xc001d26d80}, 0xc001cbf2e0, 0xc001d26fc0)                                     │
│     github.com/cs3org/reva/[email protected]/internal/grpc/interceptors/auth/auth.go:160 +0xa06                                                                         │
│ github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func2.1({0x44a27d8?, 0xc001d7f9b0?}, {0x42a2b20?, 0xc001d26d80?})                                                                │
│     github.com/grpc-ecosystem/[email protected]/chain.go:48 +0x4f                                                                                                                      │
│ github.com/grpc-ecosystem/go-grpc-middleware/recovery.UnaryServerInterceptor.func1({0x44a27d8?, 0xc001d7f9b0?}, {0x42a2b20?, 0xc001d26d80?}, 0xc001f4d270?, 0x496b57?)                         │
│     github.com/grpc-ecosystem/[email protected]/recovery/interceptors.go:33 +0xc4                                                                                                      │
│ github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func2.1({0x44a27d8?, 0xc001d7f9b0?}, {0x42a2b20?, 0xc001d26d80?})                                                                │
│     github.com/grpc-ecosystem/[email protected]/chain.go:48 +0x4f                                                                                                                      │
│ github.com/cs3org/reva/v2/internal/grpc/interceptors/log.NewUnary.func1({0x44a27d8, 0xc001d7f9b0}, {0x42a2b20, 0xc001d26d80}, 0xc001cbf2e0, 0xc001d27040)                                      │
│     github.com/cs3org/reva/[email protected]/internal/grpc/interceptors/log/log.go:39 +0x9a                                                                             │
│ github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func2.1({0x44a27d8?, 0xc001d7f9b0?}, {0x42a2b20?, 0xc001d26d80?})                                                                │
│     github.com/grpc-ecosystem/[email protected]/chain.go:48 +0x4f                                                                                                                      │
│ github.com/cs3org/reva/v2/internal/grpc/interceptors/useragent.NewUnary.func1({0x44a27d8, 0xc001d7f920}, {0x42a2b20, 0xc001d26d80}, 0x1?, 0xc001d27080)                                        │
│     github.com/cs3org/reva/[email protected]/internal/grpc/interceptors/useragent/useragent.go:38 +0xf4                                                                 │
│ github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func2.1({0x44a27d8?, 0xc001d7f920?}, {0x42a2b20?, 0xc001d26d80?})                                                                │
...

@dragonchaser
Copy link

this is covered by #3964, closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dragonchaser dragonchaser dragonchaser left review comments

@kobergj kobergj kobergj requested changes

@labkode labkode Awaiting requested review from labkode labkode will be requested when the pull request is marked ready for review labkode is a code owner

@glpatcern glpatcern Awaiting requested review from glpatcern glpatcern will be requested when the pull request is marked ready for review glpatcern is a code owner

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

Successfully merging this pull request may close these issues.
4 participants
@dragotin @kobergj @butonic @dragonchaser