Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: skip the freemem test on IBMi PASE #32043

Closed
wants to merge 1 commit into from
Closed

test: skip the freemem test on IBMi PASE #32043

wants to merge 1 commit into from

Conversation

dmabupt
Copy link
Contributor

@dmabupt dmabupt commented Mar 2, 2020

On IBMi PASE, the amount of memory in use includes storage
used for memory and disks so it is possible to exceed the
amount of main storage.

So skip this test on IBMi PASE as libuv --> https://github.com/libuv/libuv/pull/2614/files#diff-564d321f5718b4ed067d21caf87d4b59

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Mar 2, 2020
On IBMi PASE, the amount of memory in use includes storage
used for memory and disks so it is possible to exceed the
amount of main storage.
@mhdawson
Copy link
Member

mhdawson commented Mar 2, 2020

@dmabupt can you provide a bit more info. freemem can be negative? I'm having trouble how that makes sense even given the info you've provided in the first post.

@dmabupt
Copy link
Contributor Author

dmabupt commented Mar 3, 2020

@dmabupt can you provide a bit more info. freemem can be negative? I'm having trouble how that makes sense even given the info you've provided in the first post.

Hello @mhdawson

The memory concept is very different on IBMi. We cannot map the "amount of free memory" to any accurate value on IBMi.

You can access this link and search keyword "Main storage size.". This is the so called "total memory" on the partitioned system.

But we can not find the "used memory" due to the "single-level storage" machanism on IBMi. We do not know if the data is on the memory or the disk right now.

The system vaule I chose is Current unprotected storage used. -- The current amount of storage in use for temporary objects.. This indicates the amount of memory & disk used for temporary objects right now. Obviously it could exceed the total memory size.

  • Main storage size -- Total memory size
  • Current unprotected storage used -- Current used memory + disk(swap space, virtual memory, or something like that...)
  • Current Main storage used -- ??? No API

BTW, the command ps gv shows below RSS&MEM data on IBMi PASE --

$ ps gv
      PID    TTY STAT  TIME PGIN  SIZE   RSS   LIM  TSIZ   TRS %CPU %MEM COMMAND
       33      - A    11:46 1445  9688     0    xx   244     0  0.0  0.0 /QIBM/
      114      - A     0:00   64  6948     0    xx   243     0  0.0  0.0 /QOpen
   698255      - A     0:00   32  6948     0    xx   243     0  0.0  0.0 /QOpen
  3066273      - A     0:00   38  6948     0    xx   243     0  0.0  0.0 /QOpen
  4373194      - A     5:33 34786 74508     0    xx     3     0  0.0  0.0 /QOpen
  6157802      - A     0:00   41 30880     0    xx 26725     0  0.0  0.0 /QOpen
  9296852      - A     0:01 5310  5716     0    xx  1300     0  0.0  0.0 /QOpen

@mhdawson
Copy link
Member

mhdawson commented Mar 3, 2020

Ok so are the values always 0 ? If so them maybe we should make the test check that is the case versus skipping it?

@dmabupt
Copy link
Contributor Author

dmabupt commented Mar 6, 2020

Ok so are the values always 0 ? If so them maybe we should make the test check that is the case versus skipping it?

The ps command on PASE choose not to provide any memory information because we can not get the accurate data.
The API I chose in libuv provides a nearly real used memory info, althought it contains disk usages. -- Current unprotected storage used. -- The current amount of storage in use for temporary objects.

@kadler, what's your opinion?

@mhdawson
Copy link
Member

mhdawson commented Mar 6, 2020

From the API docs:

os.freemem()#
Added in: v0.3.3
Returns: <integer>
Returns the amount of free system memory in bytes as an integer.

https://nodejs.org/docs/latest-v13.x/api/os.html#os_os_freemem

@mhdawson
Copy link
Member

mhdawson commented Mar 6, 2020

Unless system memory on IBMi maps to what you have used or what is being returned is very useful, I'd lean towards the same choice that they made for ps (ie return 0).

@kadler
Copy link

kadler commented Mar 6, 2020

@dmabupt I'm not the expert on the IBM i memory accounting - it all seems a bit murky to me, especially considering the atypical design of the system. Have you talked to Angela Newton about this?

If there's nothing directly equivalent, I agree with @mhdawson that we should just return 0. We can always revisit this later if we come up with some way to get accurate information.

@dmabupt
Copy link
Contributor Author

dmabupt commented Mar 7, 2020

Unless system memory on IBMi maps to what you have used or what is being returned is very useful, I'd lean towards the same choice that they made for ps (ie return 0).

@mhdawson , If we set the in-use memory to 0, then we have to skip this test as well -- https://github.com/libuv/libuv/blob/v1.x/test/test-get-memory.c#L42 , right?

dmabupt added a commit to dmabupt/libuv that referenced this pull request Mar 7, 2020
On IBMi PASE, the amount of memory in use includes storage
used for memory and disks. So we hard coded the amount of memory
in use to zero on IBMi based on below discussion -
nodejs/node#32043
@dmabupt
Copy link
Contributor Author

dmabupt commented Mar 7, 2020

Submitted a libuv pull request for that -- libuv/libuv#2732
I will close this PR when the libuv PR landed.

dmabupt added a commit to dmabupt/libuv that referenced this pull request Mar 9, 2020
On IBMi PASE, the amount of memory in use includes storage
used for memory and disks. So we hard coded the amount of memory
in use to zero on IBMi based on below discussion -
nodejs/node#32043
@mhdawson
Copy link
Member

mhdawson commented Mar 9, 2020

@dmabupt instead of skipping we can validate that it is 0, right?

@dmabupt
Copy link
Contributor Author

dmabupt commented Mar 9, 2020

@dmabupt instead of skipping we can validate that it is 0, right?

@mhdawson You mean in the libuv code, add a validation like this ?

#ifdef __PASE__
  ASSERT(total_mem == free_mem);
#else
  ASSERT(total_mem > free_mem);
#endif

Libuv does not provide API to get the memory in use, so we can only validate that total_mem == free_mem

@mhdawson
Copy link
Member

mhdawson commented Mar 9, 2020

In the context of this PR. Instead of excluding the test with

  // On IBMi PASE, the amount of memory in use includes storage used for
  // memory and disks so it is possible to exceed the amount of main storage.
  if (!common.isIBMi)
    assert.ok(os.freemem() > 0);

more like

  // On IBMi PASE, there is no equivalent so freemem always returns 0
  if (common.isIBMi) {
    assert.ok(os.freemem() === 0);
  } else {
    assert.ok(os.freemem() > 0);
  }

@dmabupt
Copy link
Contributor Author

dmabupt commented Mar 9, 2020

In the context of this PR. Instead of excluding the test with

  // On IBMi PASE, the amount of memory in use includes storage used for
  // memory and disks so it is possible to exceed the amount of main storage.
  if (!common.isIBMi)
    assert.ok(os.freemem() > 0);

more like

  // On IBMi PASE, there is no equivalent so freemem always returns 0
  if (common.isIBMi) {
    assert.ok(os.freemem() === 0);
  } else {
    assert.ok(os.freemem() > 0);
  }

As we discussed before, we hard-coded the in-use memory to zero, not free memory. So we can only validate free-memory === total-memory.

@mhdawson
Copy link
Member

mhdawson commented Mar 9, 2020

@dmabupt ok if that is what you can check. My main point is instead of skipping, validate what makes sense.

@dmabupt
Copy link
Contributor Author

dmabupt commented Mar 9, 2020

@dmabupt ok if that is what you can check. My main point is instead of skipping, validate what makes sense.

Setting in-use memory to zero, Node.js's test code needs no change. We only need to update one line in the libuv test, which has the total_mem > free_mem test.

dmabupt added a commit to dmabupt/libuv that referenced this pull request Mar 9, 2020
On IBMi PASE, the amount of memory in use includes storage
used for memory and disks. So we hard coded the amount of memory
in use to zero on IBMi based on below discussion -
nodejs/node#32043
@dmabupt
Copy link
Contributor Author

dmabupt commented Mar 9, 2020

@dmabupt ok if that is what you can check. My main point is instead of skipping, validate what makes sense.

@mhdawson I have updated the libuv's PR. You may review it here -- libuv/libuv#2732

And if no problem, when this libuv PR landed, I will close this node.js PR (no need to change Node.js code)

bnoordhuis pushed a commit to libuv/libuv that referenced this pull request Mar 10, 2020
On IBMi PASE, the amount of memory in use includes storage used for
memory and disks. So we hard-code the amount of memory in use to zero
on IBMi, based on discussion in nodejs/node#32043.

PR-URL: #2732
Refs: nodejs/node#32043
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@dmabupt
Copy link
Contributor Author

dmabupt commented Mar 10, 2020

Close this PR since the Libuv PR libuv/libuv#2732 landed.

@dmabupt dmabupt closed this Mar 10, 2020
@dmabupt dmabupt deleted the ibmi_skip_test-os branch March 10, 2020 13:25
@richardlau richardlau mentioned this pull request Jun 20, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants