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

Optimize pub/sub key management and session ID handling #682

Merged
merged 8 commits into from
Jan 27, 2025

Conversation

cgalibern
Copy link
Contributor

Description

This pull request introduces multiple improvements and optimizations for the pubsub, daemonapi, imon, and scheduler components:

  • Key Management Optimization: The pubsub module has undergone significant refactoring to optimize key handling, including caching publication keys using sync.RWMutex. This enhances efficiency and reduces redundant operations.
  • Publisher and Subscriber Separation: Refactored the internal structure to replace the usage of pubsub.Bus with dedicated PublishBuilder and SubscribeBuilder. This change simplifies context-based publication and subscription management.
  • Session ID Handling: Removed short-lived session IDs from publication labels to facilitate caching. A dedicated RequesterSessionID field is now introduced for session-related message structures (Exec, ExecFailed, ExecSuccess), ensuring consistent traceability while improving overall performance.
  • Traceability Improvements: Incorporated session IDs into all pubsub messages, ensuring they are managed systematically without relying on redundant labels for enhanced traceability and debugging.

These changes collectively simplify the system, reduce computational overhead, and set the foundation for future caching and optimization enhancements.

…ed labels

This change facilitates future caching of labels and keys.

The update ensures that the session ID (UUID) is incorporated into all
pubsub messages, enhancing traceability and debugging.
Both the scheduler and imon components have been updated to handle session IDs
consistently, and redundant 'sid' labels have been removed, as session IDs
are now encapsulated in the dedicated session ID field.
…ache.

This commit removes session ids from labels, to have cacheable label values.

Don't anymore use published label with SessionID or RequesterSessionID.
Labels have a cost, instead move it to Exec... structures so filter is still
possible.

This commit introduces the `RequesterSessionID` field to `Exec`, `ExecFailed`,
and `ExecSuccess` message structures. So it is still possible to filter on data
value.
Stop using pubsub.Bus, and prefer separate PublishBuilder and SubscribeBuilder.

SubFromContext and PubFromContext return SubscribeBuilder and PublishBuilder.

Publicator that implements PublishBuilder will handle the keys caching.
Replaced redundant functions and improved handling of pubKeys for publications.
Added caching for derived keys using `sync.RWMutex` to enhance efficiency.
This change simplifies and optimizes how publication keys are generated and accessed.

Now each Publicator own a cache of publication filters
Extracted cmdPub construction logic into a new helper method `cmdPubFactory`,
improving readability and separation of concerns.
Adjusted the `Pub` method to handle channel communication cleanly and utilize the helper for creating `cmdPub` instances.
Replaced `Publicator` with direct use of `Bus` for publishing, removing
unnecessary abstraction.
Optimized label combination generation with a more efficient approach
using `combinations` function.

Simplified code and improved maintainability by eliminating redundant
locking and factory methods.

Refactor pubsub interfaces for clearer and unified naming

Rename interfaces `PublishBuilder` and `SubscribeBuilder` with
`Publisher` and `Subscriber`.
@cgalibern cgalibern merged commit 2f3cd98 into opensvc:main Jan 27, 2025
1 check passed
@cgalibern
Copy link
Contributor Author

From:
sample-loop-50obj-1-450s            | Type: cpu            | Duration: 450s, Total samples = 39.61s ( 8.80%)    | Showing nodes accounting for 15840ms, 39.99% of 39610ms total
To:
sample-loop-50obj-1-450s            | Type: cpu            | Duration: 450s, Total samples = 32.20s ( 7.16%)    | Showing nodes accounting for 12320ms, 38.26% of 32200ms total

New extract:

File: om
Type: cpu
Time: Jan 27, 2025 at 8:01pm (CET)
Duration: 450s, Total samples = 32.20s ( 7.16%)
Entering interactive mode (type "help" for commands, "o" for options)
Showing nodes accounting for 12320ms, 38.26% of 32200ms total
Dropped 1557 nodes (cum <= 161ms)
Showing top 10 nodes out of 432
      flat  flat%   sum%        cum   cum%
    5120ms 15.90% 15.90%     5120ms 15.90%  runtime/internal/syscall.Syscall6
    1980ms  6.15% 22.05%     1980ms  6.15%  crypto/internal/bigmod.addMulVVW2048
    1110ms  3.45% 25.50%     1140ms  3.54%  encoding/json.stateInString
     760ms  2.36% 27.86%     3490ms 10.84%  runtime.mallocgc
     690ms  2.14% 30.00%      690ms  2.14%  runtime.futex
     620ms  1.93% 31.93%      620ms  1.93%  runtime.nextFreeFast (inline)
     590ms  1.83% 33.76%      590ms  1.83%  runtime.memmove
     560ms  1.74% 35.50%     1380ms  4.29%  encoding/json.checkValid
     450ms  1.40% 36.89%     1310ms  4.07%  runtime.selectgo
     440ms  1.37% 38.26%      440ms  1.37%  runtime.memclrNoHeapPointers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant