Conversation
67f4482 to
28f0897
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3572 +/- ##
==========================================
- Coverage 26.66% 26.59% -0.07%
==========================================
Files 654 655 +1
Lines 49374 49742 +368
==========================================
+ Hits 13164 13228 +64
- Misses 35164 35470 +306
+ Partials 1046 1044 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
28f0897 to
146a50e
Compare
| nm, err := cp.netState.NetMap() | ||
| if err != nil { | ||
| cp.log.Error("could not get netmap for Container contract update", zap.Stringer("cid", ctx.cID), zap.Error(err)) | ||
| l.Error("could not get netmap for Container contract update", zap.Error(err)) |
There was a problem hiding this comment.
i'd separate refactoring from message/field addition
There was a problem hiding this comment.
can you elaborate, please? the commit says improve container creation and list updating logging i meant adding CID too
added more commits since @roman-khimov also did not like it. hope i understood the suggestion
There was a problem hiding this comment.
there was no need to change this line to add debug messages. Also thinking of #3409
There was a problem hiding this comment.
On #3409 — I had this thought too, but container management operations are not comparable to object management operations, they're rare. Even pushing it to the limits in synthetic tests can't reach the same level of ops/s we have for objects.
There was a problem hiding this comment.
yeah, i did this while remembering the issue and did understand that this happens not even every epoch. in general, i do like zap.With option in the code, if it is not a hot part. i find it less error-prone
pkg/morph/client/container/load.go
Outdated
| NumberOfObjects uint64 | ||
| } | ||
|
|
||
| // FromStackItem implemetns [stackitem.Convertible]. |
There was a problem hiding this comment.
as i already mentioned, i dont understand whats the reason to follow this interface
There was a problem hiding this comment.
well, it does what we need, it converts VM type to go struct, that is descriptive. i do not insist, but neo-go dev found it usefull, i do not mind
There was a problem hiding this comment.
at least i have no idea why it's exported. Ok, not a big deal
146a50e to
02cd46f
Compare
02cd46f to
8375c70
Compare
| nm, err := cp.netState.NetMap() | ||
| if err != nil { | ||
| cp.log.Error("could not get netmap for Container contract update", zap.Stringer("cid", ctx.cID), zap.Error(err)) | ||
| l.Error("could not get netmap for Container contract update", zap.Error(err)) |
There was a problem hiding this comment.
On #3409 — I had this thought too, but container management operations are not comparable to object management operations, they're rare. Even pushing it to the limits in synthetic tests can't reach the same level of ops/s we have for objects.
8375c70 to
6ecb994
Compare
d890878 to
9d3c767
Compare
| if np.forceContainersListUpdate.Load() { | ||
| l.Info("forcing Container contract lists update...") | ||
|
|
||
| // fixing missing container lists from 0.45.0 release; for 0.49.0 only, |
There was a problem hiding this comment.
What's gonna be the behavior after the update but before epoch tick? Failure to submit estimations?
There was a problem hiding this comment.
no, if the update was successful, the lists are filled it is possible to report. but IRs must be updated at about the same time and ofc before SN updates
There was a problem hiding this comment.
my bad, yes, only a tick will update the lists
There was a problem hiding this comment.
in the first message, i explained my first thought (unimplemented currently). do it at the start of every IR?
There was a problem hiding this comment.
It's better for upgrades, especially given that is has to be done once.
20e8cc7 to
9ca724d
Compare
fa63a7a to
07962b9
Compare
|
Needs to be rebased once again. |
This supports nspcc-dev/neofs-contract#412. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Regression starting from b6689c3. For some reason, in local installation with 1 out of 1 Alphabet signature it did not fail with contract panic (but the contract do check for Alphabet signature) and there were no messages about a failed call but still the contract was not updated and no SNs could be found. Commit also adds one-time migration if there is any empty container list in the network. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Add start and success messages. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Where will not be estimations in the next contract release. Refs nspcc-dev/neofs-contract#510. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
The old ones were dropped, there is no need in "new" suffixes. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Add support for quotas and total container and account sizes from nspcc-dev/neofs-contract#514. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Refs nspcc-dev/neofs-contract#514. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Allows inspecting result from nspcc-dev/neofs-contract#438. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
This was solved inside `List` method. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Also, update SDK to the latest version. Closes #3520. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
07962b9 to
79c5102
Compare
No description provided.