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

Recursive Power Calculation #18048

Open
xarmin-dev opened this issue Nov 20, 2024 · 3 comments
Open

Recursive Power Calculation #18048

xarmin-dev opened this issue Nov 20, 2024 · 3 comments
Assignees
Labels
complexity: medium Requires a substantial but not unusual amount of effort to implement status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application

Comments

@xarmin-dev
Copy link

NetBox version

v4.1.6

Feature type

Change to existing functionality

Triage priority

I volunteer to perform this work (if approved)

Proposed functionality

I am proposing an alternative solution to previously proposed recursive power calculations, that does not require database modification or Many-to-Many models, and is hopefully less complex in nature.

I recently delved into the NetBox code after using it at my MSP business for a year or so, so apologies for any incorrect nomenclature in the below.

Here is proposed simplified pseudocode, modifying the get_power_draw function:

def get_power_draw(self, depth_context=0, powerports_context=None):

  if self.allocated_draw is not None: return self_power_utilization
  if depth_context > MAX_POWER_CALCULATION_DEPTH: return 0

  if not powerports_context: powerports_context = []

  total = 0
  powerports = self.get_downstream_powerports()

  for port in powerports:
    if powerport.device.identifier in powerports_context: break
    
    powerports_context.append(powerport.device.identifier)
    total += powerport.get_power_draw(depth_context + 1, powerports_context)['allocated']

  return total

Additionally, I propose modifying the UI to use the maximum_draw specified for the queried device in utilization calculations, rather than that of the Power Feed.

Advantages

  • User-configurable max recursion depth to address performance concerns.
  • Simple to understand and maintain.
  • Infinite loop protections.
  • No database changes.
  • allocated_draw can continue to be used as an override for downstream power calculations.

Disadvantages

  • Increased performance impact.

I am happy to make these changes and submit a pull request. It seems that this is a fairly common request in the community, and it would certainly help my own business as well.

Previous Issues (I am sure I missed some!)

Use case

Many environments outside of a datacenter are likely to use a PDU, connected to a UPS, connected to a Power Feed. Power is not accurately modeled in these situations, and can be confusing to users.

Allowing at least one deeper level of power calculation would allow NetBox users to more accurately determine and plan power usage across PDU, UPS, and Power Feeds.

Database changes

None

External dependencies

None

@xarmin-dev xarmin-dev added status: needs triage This issue is awaiting triage by a maintainer type: feature Introduction of new functionality to the application labels Nov 20, 2024
@bctiemann bctiemann removed the status: needs triage This issue is awaiting triage by a maintainer label Nov 22, 2024
@bctiemann bctiemann added status: accepted This issue has been accepted for implementation complexity: medium Requires a substantial but not unusual amount of effort to implement labels Nov 22, 2024 — with Linear
@bctiemann
Copy link
Contributor

Seems like a good approach, clear and simple (and would address a lot of piecemeal bugs as noted). Please be sure there are adequate unit tests covering loop conditions, and if we can get some statistics on what kind of performance impact this causes that would also be a good idea.

@xarmin-dev
Copy link
Author

Thanks @bctiemann! I'll get started on this, and make sure we get that coverage and those statistics.

@xarmin-dev
Copy link
Author

I have some initial results from performance testing. I created:

  • 2 x 3-Phase Power Feeds, associated with a Rack
  • 2 x UPS-like devices, one connected to each Power Feed
  • 2 x PDU-like devices, one connected to each UPS
  • 72 x Server-like devices, 36 connected to each PDU

I observed the following, using the development branch of NetBox, the new recursion function set to a max depth of one, and the same set to a max depth of three.

(Load Time | SQL Time | Queries)

From Rack Template

  • Development Branch: 2414ms | 85ms | 40 queries
  • Recursive, MAX_DEPTH=1: 1142ms | 123ms | 59 queries
  • Recursive, MAX_DEPTH=3: 3319ms | 339ms | 205 queries

From UPS Device Template

  • Development Branch: 663ms | 57ms | 35 queries
  • Recursive, MAX_DEPTH=1: 748ms | 88ms | 35 queries
  • Recursive, MAX_DEPTH=3: 2025ms | 192ms | 110 queries

From PDU Device Template

  • Development Branch: 1165ms | 65ms | 39 queries
  • Recursive, MAX_DEPTH=1: 680ms | 62ms | 39 queries
  • Recursive, MAX_DEPTH=3: 1838ms | 169ms | 106 queries

These times vary 200-300ms each page load. I am doubtful there is a way to make any significant optimizations without adding significant complexity.

I can continue working on this, but would be curious to hear thoughts as to whether this is an acceptable performance impact for the functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity: medium Requires a substantial but not unusual amount of effort to implement status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

No branches or pull requests

2 participants