Skip to content
This repository has been archived by the owner on Dec 4, 2023. It is now read-only.

Correct address allocation equality. #3083

Merged

Conversation

k-wall
Copy link
Member

@k-wall k-wall commented Aug 9, 2019

This defect meant that all existing addresses were considered modified even if they weren't.
This lead to unnecessary reconcilations against all routers and brokers.

This defect meant that all existing addresses were consider modified even if they weren't.
This lead to unnecessary reconcilations against all routers and brokers.
@k-wall k-wall requested a review from vbusch August 9, 2019 16:48
@k-wall
Copy link
Member Author

k-wall commented Aug 9, 2019

During my test cases with large numbers of addresses and routers, the system came unresponsive and probes began to fail. Node application heap used stayed reasonable, however.

@k-wall
Copy link
Member Author

k-wall commented Aug 9, 2019

Fix: #3081

@k-wall
Copy link
Member Author

k-wall commented Aug 9, 2019

@enmasse-ci run tests

Copy link
Contributor

@vbusch vbusch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same_allocation looks good.


setInterval(() => {
log.info("Heap statistics : %j", v8.getHeapStatistics());
}, 60000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is printing the heap every minute, temporary? And we can remove it once we know why it is getting so large?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm inclined to keep it, though possible at a reduced frequency. I'm not a node expert, but I couldn't find any external method to get the virtual machine's state at run time so was rather blind. Having some heap statistics over time helps.

@k-wall k-wall merged commit 228fa4e into EnMasseProject:master Aug 12, 2019
lulf pushed a commit that referenced this pull request Aug 23, 2019
This defect meant that all existing addresses were considered modified even if they weren't.
This lead to unnecessary reconciliations against all routers and brokers, consuming extra resources across the system.

Fix: #3081
lulf pushed a commit that referenced this pull request Aug 23, 2019
* Correct address allocation equality. (#3083)

This defect meant that all existing addresses were considered modified even if they weren't.
This lead to unnecessary reconciliations against all routers and brokers, consuming extra resources across the system.

Fix: #3081

* Turn down noisy per reconcilation per address logging (#3085)

* Restrict the number of active promises when creating/deleting entities on the brokers

Fix #3090

* Restrict the number of active promises when creating/deleting entities on the routers (#3095)

Fix #3090

* Fix #3086: Adjust node max_old_space_size to a percentage of available container memory (#3099)

* Correct address status equality check (#3094)

* Fix #3089: Correct address status equality check

* address review comments
* made same_allocation same implementation symmetric

* Fix #3092: Turn off EventEmitter#maxListeners check for router management connection (#3093)

* Ensure that addresses are not synced until addresses have been defined

As proposed in #3101

* Fix unit tests

* Start the watcher after the listeners are in place

Signed-off-by: Vanessa <vbusch@redhat.com>

* Improve api server address create performance (#3114)

This change improves api server address create performance by almost an order of magnitude when 1000 addresses are defined.
The changes moves the validation of spec.address to standard-controller for the standard address space. For the brokered address space, the validation remains in the api-server, as it would require adding write-back capability to the agent. Future refactoring/consolidation of agent/standard-controller should incorporate this validation.

Fixes #3111
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants