Skip stats under error conditions. #218
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gab-arrobo
reviewed
Aug 29, 2024
stats/stats.go
Outdated
| x := m.T.Sub(ue.CReg.RegReqOutTime) | ||
| ue.CReg.RegReqAuthReq = x.Microseconds() | ||
| logger.StatsLog.Infoln("Time[us] between Reg Req & Auth Req ", ue.CReg.RegReqAuthReq) | ||
| if t != nil { |
There was a problem hiding this comment.
Do you know why the transaction (Id) does not already exist in the StatisticsEvent?
There was a problem hiding this comment.
After further debugging, I see that the same transaction (Id=14) gets "popped" twice in the stats, which leads to nil pointer. Transaction 14 "comes" from HandleInitialContextSetupRequest (gnodeb/worker/gnbcpueworker/handler.go) and from HandleServiceAcceptEvent (simue/handler.go). Does it mean that the same output "event" generates 2 "input" events/messages?
Received Event: PDU_SESS_ACC_IN, Id: 9
Received Event: UE_CTX_CMD_IN, Id: 12
Received Event: ICS_REQ_IN, Id: 14
Received Event: SVC_ACCEPT_IN, Id: 14
There was a problem hiding this comment.
Yes current design needs some changes to handle few corner cases and hence it crashes if transaction not found. I would say at least gnbsim should not crash if no transaction found.
gab-arrobo
reviewed
Aug 30, 2024
Signed-off-by: Ajay Lotan Thakur <thakur.ajay@gmail.com>
thakurajayL
force-pushed
the
dev-stats-fix
branch
from
57ccb86 to
d42963e
Compare
gab-arrobo
approved these changes
Sep 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some of the messags are nested and they need to have multiple statistics Id assigned to them. This is advance scenario which needs further design. But current design is good enough for most of the common scenario. Hence I have proposed to avoid POD crahshes if stats Id is not found.