TrueCloudLab/frostfs-node
20
1
Fork 27
Code Issues 100 Pull requests 4 Projects Releases 1 Activity Actions

Upd/neo-go subscriptions #73

Merged
carpawell merged 2 commits from carpawell/upd/neo-go-subs into master 2023-03-24 09:42:31 +00:00
Conversation 1 Commits 2 Files changed 4 +210 -117
carpawell commented 2023-02-22 13:08:38 +00:00 (Migrated from github.com)
Copy link

Only adopts new API. More adds that removes; could look strange a little. But suggest separating updates and refactors.

Only adopts new API. More adds that removes; could look strange a little. But suggest separating updates and refactors.
ale64bit (Migrated from github.com) reviewed 2023-02-22 13:08:38 +00:00
dstepanov-yadro (Migrated from github.com) reviewed 2023-02-22 13:08:38 +00:00
fyrchik (Migrated from github.com) reviewed 2023-02-22 16:37:58 +00:00
fyrchik (Migrated from github.com) left a comment
Copy link

Correct me if I am wrong, in this PR our main goal is to remove some of the deprecated messages?

Correct me if I am wrong, in this PR our main goal is to remove some of the `deprecated` messages?
pkg/morph/client/multi.go Outdated
fyrchik (Migrated from github.com) commented 2023-02-22 16:29:43 +00:00
Copy link

?

?
pkg/morph/client/multi.go Outdated
fyrchik (Migrated from github.com) commented 2023-02-22 16:29:56 +00:00
Copy link

??

??
pkg/morph/client/notifications.go Outdated
fyrchik (Migrated from github.com) commented 2023-02-22 16:31:11 +00:00
Copy link

If it is false, do can we take a mutex too?
It could look more readable without if.

If it is false, do can we take a mutex too? It could look more readable without `if`.
carpawell (Migrated from github.com) reviewed 2023-02-27 12:33:13 +00:00
carpawell (Migrated from github.com) left a comment
Copy link

Correct me if I am wrong, in this PR our main goal is to remove some of the deprecated messages?

@fyrchik, yes, all the "deprecated" msgs about the notification processes are fixed (but that is not a refactor and that is not a PR that fixes all the linter messages)

> Correct me if I am wrong, in this PR our main goal is to remove some of the deprecated messages? @fyrchik, yes, all the "deprecated" msgs about the notification processes are fixed (but that is not a refactor and that is not a PR that fixes _all_ the linter messages)
carpawell (Migrated from github.com) reviewed 2023-02-27 12:33:45 +00:00
pkg/morph/client/notifications.go Outdated
carpawell (Migrated from github.com) commented 2023-02-27 12:33:45 +00:00
Copy link

no, we would have a dead-lock then: background flag is false only if we have already lost connection and try to establish a new one, so switchLock has been already taken

no, we would have a dead-lock then: `background` flag is `false` only if we have already lost connection and try to establish a new one, so `switchLock` has been already taken
carpawell (Migrated from github.com) reviewed 2023-02-27 12:34:52 +00:00
pkg/morph/client/multi.go Outdated
carpawell (Migrated from github.com) commented 2023-02-27 12:34:52 +00:00
Copy link

could not find unlocking line while was preparing that PR so decide to chose a better (IMO) spacing

could not find unlocking line while was preparing that PR so decide to chose a better (IMO) spacing
carpawell (Migrated from github.com) reviewed 2023-02-27 12:37:32 +00:00
pkg/morph/client/multi.go Outdated
carpawell (Migrated from github.com) commented 2023-02-27 12:37:32 +00:00
Copy link

a separate commit/keep as it was?

a separate commit/keep as it was?
fyrchik (Migrated from github.com) approved these changes 2023-02-28 10:44:04 +00:00
pkg/morph/client/client.go Outdated
fyrchik (Migrated from github.com) commented 2023-02-28 10:43:15 +00:00
Copy link

Can we embed subsInfo here? It allows us to get rid of updateSubs and possibly prevents us from forgetting something to copy when we add it.

Can we embed `subsInfo` here? It allows us to get rid of `updateSubs` and possibly prevents us from forgetting something to copy when we add it.
pkg/morph/client/multi.go Outdated
fyrchik (Migrated from github.com) commented 2023-02-28 10:41:17 +00:00
Copy link

I would keep if it then linter doesn't complain.

I would keep if it then linter doesn't complain.
acid-ant (Migrated from github.com) approved these changes 2023-02-28 10:57:24 +00:00
carpawell added 72 commits 2023-03-23 21:25:44 +00:00 
Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
Compute shard hash only once

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Update HRW lib and use typed HRW methods to sort shards and nodes

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>
Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>
Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>
Due to source code relocation from GitHub.

Signed-off-by: Alex Vanin <a.vanin@yadro.com>
It will prevent test fails with `-race` flag on components that have
background processes and make some actions on test framework.

Signed-off-by: Pavel Karpy <p.karpy@yadro.com>
Signed-off-by: Pavel Karpy <p.karpy@yadro.com>
Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>
Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>
Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
If `processNodes` exits earlier for some reason, `needLocalCopy` could
be false.
See https://github.com/nspcc-dev/neofs-node/issues/2267

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
In previous implementation pretty-printer of basic ACL in NeoFS CLI had
mistakes:
 * F-bit was set to `Extendable()` property instead of its inversion
 * B-bits were set to `acl.RoleInnerRing` rights

Make `PrettyPrintTableBACL` to correctly render mentioned bits.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
Issue templates are still supported by Gitea:
https://docs.gitea.io/en-us/issue-pull-request-templates/ .

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
Skip deprecated warning for now, adopting new neo-go API will be done in
another task.

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>
Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>
Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
Remember the last synchronized height and use it after service restart.

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
Add make targets to simplify pre-commit setup for individual developers.

Signed-off-by: Stanislav Bogatyrev <realloc@realloc.spb.ru>
Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>
Extract assemble logic to assembler

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Add unit tests to cover all assemble statements

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Should return an error in case of a broken LOB reference chain.

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Original shellcheck action requires Docker to run and it's not always
available, especially inside Docker containers. Replacing it with python
wrapper to simplify usage with Docker-based CI systems like Drone and
WoodpeckerCI.

Signed-off-by: Stanislav Bogatyrev <s.bogatyrev@yadro.com>
Signed-off-by: Alejandro Lopez <a.lopez@yadro.com>
Previously a token could've expired in the middle of an object.PUT
stream, leading to upload being interrupted. This is bad, because user
doesn't always now what is the right values for the session token
lifetime. More than that, setting it to a very high value will
eventually blow up the session token database.

In this commit we read the session token once and reuse it for the whole
stream duration.

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
It will allow reusing that method in expiration checks.

Signed-off-by: Pavel Karpy <p.karpy@yadro.com>
Allow replication of any (expired too) locked object. Information about
object locking is considered to be presented on the _container nodes_.

Signed-off-by: Pavel Karpy <p.karpy@yadro.com>
May make docker builds not so clean.

Signed-off-by: Stanislav Bogatyrev <s.bogatyrev@yadro.com>
Signed-off-by: Pavel Karpy <p.karpy@yadro.com>
This reverts commit 2567f8020e. It assumes
that assembling logic could break some failover scenarios if request
forwarding is done. However, it also breaks requesting big objects via a
non-container node with TTL=2. Failover has been rechecked without that
commit and no problems were found. Any (if found) other bugs related to
the forwarding and object assembling must be solved more carefully.

Signed-off-by: Pavel Karpy <p.karpy@yadro.com>
Signed-off-by: Artem Tataurov <a.tataurov@yadro.com>
In our policy we mark commits not having a PR/Issue yet with a `[#XX]`
reference to be replaced after PR creation.

Signed-off-by: Stanislav Bogatyrev <s.bogatyrev@yadro.com>
Minor changes to see what fails first

Signed-off-by: Stanislav Bogatyrev <s.bogatyrev@yadro.com>
Until #139 is fixed, we can't use root inside Docker container running
CI, but Woodpecker CI can't run non-root containers until they fix
https://github.com/woodpecker-ci/woodpecker/issues/1077, hence we use
temporary kludges with custom image and manual permissions in pipelines.

Signed-off-by: Stanislav Bogatyrev <s.bogatyrev@yadro.com>
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Context has to be passed as an argument: https://pkg.go.dev/context

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Long functions are hard to understand and source of errors

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Code with high cognitive complexity is hard intuitively to understand

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
[#148] linter: Add contextcheck linter
Some checks failed
ci/woodpecker/push/full-pre-commit Pipeline failed
Details
ci/woodpecker/push/pre-commit Pipeline was successful
Details
44b86bac5a 
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
[#152] IR: Process empty basic incomes
Some checks failed
ci/woodpecker/push/full-pre-commit Pipeline failed
Details
ci/woodpecker/push/pre-commit Pipeline was successful
Details
9cd8f7cea0 
If network is not configured for basic income earnings, do not distribute
GAS by the Alphabet nodes.

Signed-off-by: Pavel Karpy <p.karpy@yadro.com>
Added unit test that verifies that GC deletes expired
locked objects in one epoch.

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
GC deletes expired locks and objects sequentially. Expired locks and
objects are now being deleted concurrently in batches. Added a config
parameter that controls the number of concurrent workers and batch size.

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
[#145] docs: Add expired object collector params
Some checks failed
ci/woodpecker/pr/full-pre-commit Pipeline failed
Details
ci/woodpecker/pr/pre-commit Pipeline was successful
Details
ci/woodpecker/push/full-pre-commit Pipeline failed
Details
ci/woodpecker/push/pre-commit Pipeline was successful
Details
7a31988a36 
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
[#128] IR: Do not try to emit GAS to nobody
Some checks failed
ci/woodpecker/push/full-pre-commit Pipeline failed
Details
ci/woodpecker/push/pre-commit Pipeline was successful
Details
9a4f40626c 
Fix sending GAS to an empty extra wallets receivers list. Also, send GAS to
extra wallets even if netmap is empty.

Signed-off-by: Pavel Karpy <p.karpy@yadro.com>
[#153] ci: Minor pipeline fixes
All checks were successful
ci/woodpecker/push/pre-commit Pipeline was successful
Details
9aeea0b974 
- We can skip full pre-commit run
- On a very slow agent golangci run may take up to 10 minutes

Signed-off-by: Stanislav Bogatyrev <s.bogatyrev@yadro.com>
Signed-off-by: Liza <e.chichindaeva@yadro.com>
Add to Possible Solutions
All checks were successful
ci/woodpecker/pr/pre-commit Pipeline was successful
Details
ci/woodpecker/push/pre-commit Pipeline was successful
Details
ec2c5d45b4 
Signed-off-by: Liza <e.chichindaeva@yadro.com>
Initially it was there to check whether an update is being initiated by
a proper node. It is now obsolete for 2 reasons:
1. Background synchronization fetches all operations from a single node.
2. There are a lot more problems with trust in the tree service, it is
   only used in controlled environments.

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
[#156] object/get: Make toHeadPrm() pass gocognit
All checks were successful
ci/woodpecker/push/pre-commit Pipeline was successful
Details
cbc2efb1d6 
Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
In case of session token (ST) with object IDs search should
return only objects allowed in static session

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
[#155] search-service: Fix search with ST
All checks were successful
ci/woodpecker/push/pre-commit Pipeline was successful
Details
49234b915e 
Search should return only objects allowed in static session

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
[#98] fstree: Do not fail iteration over just removed files
All checks were successful
ci/woodpecker/push/pre-commit Pipeline was successful
Details
da8da1c63a 
A directory is read and files are saved to a local variable. The iteration
over such files may lead to a non-existing files reading due to a normal SN
operation cycle and, therefore, may lead to a returning the OS error to a
caller. Skip just removed (or lost) files as the golang std library does in
similar situations:
5f1a0320b9/src/os/dir_unix.go (L128-L133).

Signed-off-by: Pavel Karpy <p.karpy@yadro.com>
[#158] Fix gitlint run in pre-commit under CI
All checks were successful
ci/woodpecker/push/pre-commit Pipeline was successful
Details
f111704ceb 
Wrong stage was used.
It still may run gitlint against only latest commit in PR.

Signed-off-by: Stanislav Bogatyrev <s.bogatyrev@yadro.com>
[#159] Add handle __SYSTEM__ sys attributes
All checks were successful
ci/woodpecker/pr/pre-commit Pipeline was successful
Details
ci/woodpecker/push/pre-commit Pipeline was successful
Details
342e571d89 
Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>
[#86] node: Move testing utils to one package
Some checks failed
ci/woodpecker/push/pre-commit Pipeline failed
Details
9808dec591 
Move testing utils from tests in local_object_storage package to
unified testutil package

Signed-off-by: Airat Arifullin <aarifullin@yadro.com>
[#86] node: Fix unit test and linter errors
All checks were successful
ci/woodpecker/pr/pre-commit Pipeline was successful
Details
ci/woodpecker/push/pre-commit Pipeline was successful
Details
34329d67ff 
Signed-off-by: Airat Arifullin <aarifullin@yadro.com>
carpawell commented 2023-03-23 21:26:21 +00:00
Contributor
Copy link

Can we embed subsInfo here? It allows us to get rid of updateSubs and possibly prevents us from forgetting something to copy when we add it.

@fyrchik, fixed

> Can we embed subsInfo here? It allows us to get rid of updateSubs and possibly prevents us from forgetting something to copy when we add it. @fyrchik, fixed
carpawell added the due date 2023-03-24 2023-03-23 23:14:14 +00:00
carpawell requested review from aarifullin 2023-03-23 23:15:04 +00:00
carpawell requested review from storage-core-developers 2023-03-23 23:15:31 +00:00
carpawell requested review from storage-core-committers 2023-03-23 23:15:43 +00:00
carpawell force-pushed carpawell/upd/neo-go-subs from 1189ac6874 to 7a7573bbe4 2023-03-24 00:28:52 +00:00 Compare
carpawell force-pushed carpawell/upd/neo-go-subs from 7a7573bbe4 to 8c092eebba 2023-03-24 00:30:56 +00:00 Compare
fyrchik approved these changes 2023-03-24 07:29:08 +00:00
dstepanov-yadro approved these changes 2023-03-24 08:49:29 +00:00
fyrchik merged commit f3ff9fd251 into master 2023-03-24 09:42:31 +00:00
fyrchik deleted branch carpawell/upd/neo-go-subs 2023-03-24 09:42:31 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
aarifullin
fyrchik
dstepanov-yadro
ale64bit
dstepanov-yadro
fyrchik
acid-ant
Labels
Clear labels   
P0

Highest

  
P1

High

  
P2

Medium

  
P3

Low

  
badger

Badger DB related issues

  
frostfs-adm

   
frostfs-cli

   
frostfs-ir

   
frostfs-lens

   
frostfs-node

   
good first issue

Good for newcomers

  
triage
  
Infrastructure

Infrastructure, Automation, CI/CD or DevOps related stuff

  
blocked

The issue or PR is blocked by something

  
bug

Something is not working

  
config

Configuration format update or breaking change

  
discussion

Talking about something in order to reach a decision or to exchange ideas

  
documentation

Documentation related

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
go

Language features adoption

  
help wanted

Extra attention is needed

  
internal

   
invalid

Something is wrong

  
kludge

This is a kludge and we know it

  
observability

Related to metrics, tracing and logs

  
perfomance

Perfomance related

  
question

More information is needed

  
refactoring

Refactoring

  
wontfix

This won't be fixed

No labels
P0
P1
P2
P3
badger
frostfs-adm
frostfs-cli
frostfs-ir
frostfs-lens
frostfs-node
good first issue
triage
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
4 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

2023-03-24

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#73
No description provided.
Delete branch "carpawell/upd/neo-go-subs"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?