Fix dmsg tracker #1238
Merged
Fix dmsg tracker #1238
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
This commit removes the dmsgtracker from Hypervisor and instead uses it from the struct Visor. Previsoly we were creating a new dmsgtracker for Visor and Hypervisor which isn't necessarry as we can access the struct Visor from the struct Hypervisor.
This commit moves the initilization of the dmsgtrackers to its own module.
This commit adds ready chan to Manager as well as the method Ready.
This commit changes the dtm method from MustGet to Get. The method MustGet blocks until it gets back the data so we use Get instead.
This commit reverts back to using MustGet in Summary as well as reverts back the changes made to GetBulk.
This commit updates the method mustEstablishTracker to not return anything but instead save the tracker directly. New ticker is also added in order to stop the goroutine containing newDmsgTracker after updateTimeout incase context.WithDeadline doesn't work.
This commit updates the method GetBulk by using the method MustGet to get the DmsgClientSummary of all the pks. Hence the method GetBulk now also returns error like MustGet and takes in ctx.
This commit updates the mustEstablishTracker method by using doneCh to aleart the select statement that it is done and prevent the warn log of timeout.
This commit uses the ctx with timeout as the timeout case instead of a ticker.
This commit fixes the dmsg-tracker test by creating a new method establishTracker that is used in method Get to run as a goroutine. The method mustEstablishTracker now returns *DmsgTracker on success or an error on failure and is used in the method MustGet.
|
@mrpalide @jdknives I would like to have your opinion on the method MustGet of DmsgTrackerManager and it's usage. It is a blocking method that blocks until it gets back a dmsgtracker. Do we really need it or we can simply run the In my opinion, Edit: Now the method |
This commit changes the method MustGet to be non-blocking and renames it to ShouldGet.
mrpalide
approved these changes
Jun 6, 2022
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.
Did you run
make format && make check?yes
Fixes #1231
Fixes #1233
Changes:
How to test this PR:
// replace github.com/skycoin/dmsg => ../dmsgingo.modmake depskywire-servicesstart a integration envdmsg-serverwithCtrl+Zdmsg-serverby getting it's PID fromhtopand killing it withsudo kill -9 <pid>dmsg-serverwithsource integration/dmsg-server-env.sh; ./bin/dmsg-server ./integration/dmsg-server.json --syslog localhost:514 --tag MSG_SRV