pull request on adding missing statistic under solaris/illumos #1381
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
1. the disk io statistic data as: nread, nwritten, reads, writes, rtime, wtime; 2. the free memory under global zone; 3. the net io statistic data as: rbytes64, ipackets64, idrops64, ierrors, obytes64, opackets64, odrops64, oerrors. The new feature branch adds the above missing statistic data based on the psutil project (https://psutil.readthedocs.io/), it has been tested under solaris ( Oracle Solaris 11.4 X86) and illumos (OmniOS v11 r151044).
shirou
reviewed
Nov 27, 2022
mem/mem_solaris.go
Outdated
| @@ -34,6 +35,13 @@ func VirtualMemoryWithContext(ctx context.Context) (*VirtualMemoryStat, error) { | |||
| return nil, err | |||
| } | |||
| result.Total = cap | |||
| freemem, err := globalZoneFreeMemory() | |||
There was a problem hiding this comment.
How about to pass ctx instead of creating a new context in this function?
Suggested change
| freemem, err := globalZoneFreeMemory() | |
| freemem, err := globalZoneFreeMemory(ctx) |
There was a problem hiding this comment.
Hi Shirou,
Thank for your review and comment; I have made the suggested change and submitted the commit.
For the tested OS environments, below is what I did for the Solaris and illumos environments:
- For the Solaris OS test, I have created the Solaris instance from the Oracle Solaris 11.4 image provided by the Oracle Cloud marketplace at https://cloudmarketplace.oracle.com/marketplace/en_US/listing/61750333 (OCI allows to create free Solaris instances with the public image);
- For the illumos OS test, I used the omniOS-r151044 (recommended by the illumos.org) from https://omnios.org/download.html to create the virtual machine and do the test.
For both environments, I tested with the go1.15 illumos/amd64 (and go1.13 illumos/amd64) and tests result look good.
|
Thank, Shirou! Will contribute the other missing features under Solaris/Illumos if needed. |
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.
The current codes miss below statistic data under solaris/illumos:
The new feature branch adds the above missing statistic data based on the psutil project (https://psutil.readthedocs.io/), it has been tested under solaris ( Oracle Solaris 11.4 X86) and illumos (OmniOS v11 r151044).