Skip to content

smartOS builds are taking more than 6 hours #4011

Closed

Description

@anonrig

Hi,

smartOS builds are taking more than 3.5 hours at the moment, and blocking landing URLPattern PR (nodejs/node#56452)

3.5 hours: https://ci.nodejs.org/job/node-test-commit-smartos/58956/nodes=smartos22-x64/console
6 hours: https://ci.nodejs.org/job/node-test-commit-smartos/58762/
7 hours and 41 minutes: https://ci.nodejs.org/job/node-test-commit-smartos/58760/

cc @jasnell @mcollina @nodejs/platform-smartos @nodejs/tsc @nodejs/build

Activity

  1. bahamat commented on Jan 31, 2025

    @bahamat

    When @ryanaslett and I were setting up these new instances, we noticed that total build time was significantly affected by caching. When the cache empty or invalidated the builds would take significantly longer (around 3+h). Builds with valid cache will take about 30-40m (IIRC).

    I don’t really understand what the caching situation is, Ryan may be able to share more details. But to my knowledge this isn’t an OS controlled behavior.

    If someone who is more familiar with the build system and the caching model is available and up to it, we can try to help debug this live to figure out what’s going on.

  2. targos commented on Jan 31, 2025

    @targos
    Member

    Merging nodejs/node#55014 completely invalidated the cache.

  3. anonrig commented on Jan 31, 2025

    @anonrig
    MemberAuthor

    There are even cases where it takes 6 hours: https://ci.nodejs.org/job/node-test-commit-smartos/58762/

  4. anonrig commented on Jan 31, 2025

    @anonrig
    MemberAuthor

    When the cache empty or invalidated the builds would take significantly longer (around 3+h). Builds with valid cache will take about 30-40m (IIRC).

    There are examples where the builds take 6 hours. Even with or without ccache, I think there is something wrong.

  5. changed the title [-]smartOS builds are taking more than 3.5 hours[/-] [+]smartOS builds are taking more than 6 hours[/+] on Jan 31, 2025
  6. bahamat commented on Jan 31, 2025

    @bahamat

    Do you have a specific build log where it took 6h with a known valid cache that I could look at? I would definitely want to start by looking at that log before attempting to replicate it.

  7. targos commented on Jan 31, 2025

    @targos
    Member

    @anonrig The number on the top-right of these pages includes the time that the job was waiting for available machines. The jobs themselves (https://ci.nodejs.org/job/node-test-commit-smartos/58762/nodes=smartos22-x64/ and https://ci.nodejs.org/job/node-test-commit-smartos/58762/nodes=smartos23-x64/) didn't take that long.

  8. bahamat commented on Jan 31, 2025

    @bahamat

    Merging nodejs/node#55014 completely invalidated the cache.

    This is useful information, and is definitely a significant contributor to what’s going on.

    How does this cache affect other builds?

  9. anonrig commented on Jan 31, 2025

    @anonrig
    MemberAuthor

    Do you have a specific build log where it took 6h with a known valid cache that I could look at? I would definitely want to start by looking at that log before attempting to replicate it.

    The closest I can find right now is URLPattern PR. It is still building for almost 4 hours and didn't start running the tests...

  10. targos commented on Jan 31, 2025

    @targos
    Member

    But I agree that more than 4 hours to build without cache doesn't seem reasonable. Are those executors on particularly old or undersized hardware?

  11. anonrig commented on Jan 31, 2025

    @anonrig
    MemberAuthor

    @anonrig The number on the top-right of these pages includes the time that the job was waiting for available machines. The jobs themselves (https://ci.nodejs.org/job/node-test-commit-smartos/58762/nodes=smartos22-x64/ and https://ci.nodejs.org/job/node-test-commit-smartos/58762/nodes=smartos23-x64/) didn't take that long.

    Yes but that shows that our smartOS machine count is not sufficient enough to handle our CI run which impacts the PR time to land. URLPattern PR shares an example where it takes more than 4 hours...

  12. bahamat commented on Jan 31, 2025

    @bahamat

    But I agree that more than 4 hours to build without cache doesn't seem reasonable. Are those executors on particularly old or undersized hardware?

    They have 8GB of RAM and 4 CPUs. If it needs to be increased, I can do that.

  13. bahamat commented on Jan 31, 2025

    @bahamat

    Yes but that shows that our smartOS machine count is not sufficient enough to handle our CI run which impacts the PR time to land. URLPattern PR shares an example where it takes more than 4 hours...

    How many runners do you suggest? We can create more.

  14. targos commented on Jan 31, 2025

    @targos
    Member

    https://ci.nodejs.org/view/All/job/node-test-commit-smartos/nodes=smartos22-x64/58957/consoleFull

    14:53:31 + ./configure
    14:53:32 Node.js configure: Found Python 3.11.1...
    14:53:34 INFO: configure completed successfully
    14:53:34 + make run-ci -j 1
    14:53:34 + JOBS=4
    14:53:34 + NODE_TEST_DIR=/home/iojs/node-tmp
    14:53:34 + NODE_COMMON_PORT=15000
    14:53:34 + FLAKY_TESTS=dontcare
    14:53:34 + TEST_CI_ARGS='-t 300'
    14:53:34 /opt/local/bin/python3.11 ./configure --verbose 
    14:53:34 Node.js configure: Found Python 3.11.1...
    14:53:34 Detected C++ compiler (CXX=ccache g++) version: 12.2.0
    14:53:34 Detected C compiler (CC=ccache gcc) version: 12.2.0
    

    It looks like only one CPU is used??? I don't know which will take precedence between -j 1 and JOBS=4

    Also, it seems that ./configure is called once for nothing before make run-ci (which calls it again)

  15. targos commented on Jan 31, 2025

    @targos
    Member

    https://ci.nodejs.org/view/All/job/node-test-commit-smartos/configure contains:

    #/bin/bash -xe
    
    curl https://raw.githubusercontent.com/nodejs/build/main/jenkins/scripts/node-test-commit-pre.sh | bash -ex -s before
    
    export CCACHE_BASEDIR=$(pwd)
    export CC="ccache gcc"
    export CXX="ccache g++"
    # Need to add this to ENV var to make behaviors match between smart os platforms: See: https://github.com/nodejs/build/issues/3731#issuecomment-2546193074
    export CXXFLAGS="${CXXFLAGS} -fno-zero-initialized-in-bss"
    curl -sLO https://raw.githubusercontent.com/nodejs/build/main/jenkins/scripts/select-compiler.sh
    . ./select-compiler.sh
    # configure will pick the correct version of Python to use on all currently supported Node.js versions. 
    # Starting from SmartOS 20 the unversioned `python` is Python 3 which doesn't work with Node.js 12.
    ./configure
    JOBS=4 NODE_TEST_DIR=${HOME}/node-tmp NODE_COMMON_PORT=15000 FLAKY_TESTS=dontcare TEST_CI_ARGS="-t 300" make run-ci -j $JOBS
    
    curl https://raw.githubusercontent.com/nodejs/build/main/jenkins/scripts/node-test-commit-diagnostics.sh | bash -ex -s after
    
  16. 17 remaining items

  17. targos commented on Jan 31, 2025

    @targos
    Member

    Here's a build against the main branch: ci.nodejs.org/job/node-test-commit-smartos/58964

    Perfect. And do you know if that has a primed cache or not?

    I don't know if you can.

  18. targos commented on Jan 31, 2025

    @targos
    Member

    I think the parallelism is working on https://ci.nodejs.org/job/node-test-commit-smartos/58964/nodes=smartos23-x64/ (by looking at the timestamps of log lines)

  19. targos commented on Jan 31, 2025

    @targos
    Member

    The build went fine (in less than 15 minutes!), so the cache was probably hot.
    But then the documentation generation failed with an out of memory error from V8.

  20. jclulow commented on Jan 31, 2025

    @jclulow

    We may need a larger RSS or swap cap on the containers. Is there a document that describes the resource usage of the build? How are the sizes and resource limits chosen for CI runners on other platforms?

  21. mhdawson commented on Jan 31, 2025

    @mhdawson
    Member

    For follow up later on I think that JOBS was being set through ansible when it configures the jenkins agent on a machine.

    In terms of resources, I don't know if that is written down, but 8G and 4CPU is in the general category if not a bit higher. I do know that @richardlau has added additional swap to some machines.

    On some machines we do see that with higher parallelism we can run out of memory with v8 using more as more threads are available. (Edit and maybe that is what happend with the doc generation as well)

    @jclulow, @bahamat if we can just up the available memory to get it running with -J4 and then investigate how to better tune that would be great.

  22. targos commented on Jan 31, 2025

    @targos
    Member

    On some machines we do see that with higher parallelism we can run out of memory with v8 using more as more threads are available.

    That's for building V8. I'm very surprised to see an error at the doc building step, which should not be done in parallel with anything else.

  23. mhdawson commented on Jan 31, 2025

    @mhdawson
    Member

    That's for building V8. I'm very surprised to see an error at the doc building step, which should not be done in parallel with anything else.

    Not sure then, worth re-running so see if it was a one-off ?

  24. targos commented on Jan 31, 2025

    @targos
    Member
  25. bahamat commented on Feb 1, 2025

    @bahamat

    In terms of resources, I don't know if that is written down, but 8G and 4CPU is in the general category if not a bit higher. I do know that @richardlau has added additional swap to some machines.

    These instances have 7GB (a consequence of running a zone in a VM) and 4 cpus. So we're in the neighborhood, although not quite where we'd like.

    @jclulow, @bahamat if we can just up the available memory to get it running with -J4 and then investigate how to better tune that would be great.

    I'm working on resizing these to give them ore resources. @jclulow and I were thinking around 12GB for the instances should do it.

  26. jclulow commented on Feb 1, 2025

    @jclulow

    And a significantly higher swap allowance as well, to account for differences in memory overcommit between Linux and illumos, and for the fact that /tmp is tmpfs/swap-backed on illumos, etc.

  27. bahamat commented on Feb 1, 2025

    @bahamat

    @targos @mhdawson @ryanaslett I need to reboot the SmartOS jenkins instances in order to apply the new memory settings.

    Is there a protocol for requesting down time?

  28. targos commented on Feb 1, 2025

    @targos
    Member

    I don't think there's more protocol than using a GitHub issue. I suggest we do it when the CI is relatively quiet. You just need someone from @nodejs/jenkins-admins to put the workers offline during the operation.

  29. jasnell commented on Feb 1, 2025

    @jasnell
    Member

    Thank you @bahamat and @jclulow for the effort on this

  30. ryanaslett commented on Mar 30, 2025

    @ryanaslett
    Contributor

    I had to rebuild some of the smartOS machines in support of another effort, and now all the machines are configured to use the available ram on the instances (not sure if that was the right way to do this).

    They are currently running in around 45 minutes which seems on par with the rest of the infrastructure.

    @bahamat / @jclulow Can you review the docs I put together #4055 ? These are the steps I've had to go through to provision a new smartos machine. Some of those steps may be able to be moved to ansible, and perhaps something can be done on the smartos side to not require a custom legacy image as well.

    Ideally we'd have a more finished base image with python etc already installed.

    I believe we can close this issue given that we've got builds under control.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions