The waitgroup counter goes up before we even try to start
the service. So, if the start fails, the counter doesn't
go back down. This messes up the graceful shutdown of
cgr-engine.
`ClientConnector` is no longer defined within `rpcclient` in its latest
version. It has been changed to be obtained from the `cgrates/birpc`
library instead.
Replaced `net/rpc` with `cgrates/birpc` and `net/rpc/jsonrpc` with
`cgrates/birpc/jsonrpc` libraries.
The implementations of `CallBiRPC()` and `Handlers()` were removed,
along with the methods associated with them.
The `rpcclient.BIRPCConector` and the methods prefixed with `BiRPC` were
removed from the `BiRPClient` interface.
The `BiRPClient` interface was renamed to `BIRPCClient`, although not
sure if needed (seems useful just to test if the structure is correct).
`rpcclient.BiRPCConector` has been replaced with `context.ClientConnector`,
which is now passed alongside `context.Context` within the same struct
(`cgrates/birpc/context.Context`). Consequently, functions that were
previously relying on it are now receiving the context instead. The
changes were made in the following functions:
- `engine/connmanager.go` - `*ConnManager.Call`
- `engine/connmanager.go` - `*ConnManager.getConn`
- `engine/connmanager.go` - `*ConnManager.getConnWithConfig`
- `engine/libengine.go` - `NewRPCPool`
- `engine/libengine.go` - `NewRPCConnection`
- `agents/libagents.go` - `processRequest`
Compilation errors related to the `rpcclient.NewRPCClient` function were
resolved by adding the missing `context`, `max_reconnect_interval`, and
`delayFunc` parameters. Additionally, context was added to all calls made
by the client. An effort was made to avoid passing hardcoded values as
much as possible, and extra flags were added where necessary for cgr
binaries.
The `max_reconnect_interval` parameter is now passed from parent
functions, which required adjustments to the function signature.
A new context field was added to all agent objects to ensure access to
it before sending it to the `connmanager's Call`, effectively replacing
`birpcclient`. Although an alternative would have been to create the
new service and add it to the context right before passing it to the
handlers, the chosen approach is definitely more comfortable.
With the addition of a context field for the SIP servers agents, an
additional error needed to be handled, coming from the creation of the
service. Agent constructors within the services package log errors as
they occur and return. Alternate solutions considered were either
shutting down the engine instead of returning, or just logging the
occurrence as a warning, particularly when the `ctx.Client` isn't
required, especially in cases where bidirectional connections are not
needed. For the latter option, it's crucial to return the object with
the error rather than nil or to make the error nil immediately after
logging.
Context has been integrated into all internal Call implementations to
ensure the objects conform to the `birpc.ClientConnector` interface.
These implementations will be removed in the near future as all service
objects are being wrapped in a `birpc.Service` type that satisfies the
`birpc.ClientConnector` interface. Currently, they are being retained
as a reference in case of any unexpected issues that arise.
Ensured that the `birpc.Service` wrapped service objects are passed to
the internal channel getters rather than the objects themselves.
Add context.TODO() to all \*ConnManager.Call function calls. To be
replaced with the context passed to the Method, when available.
For all `*ConnManager.Call` function calls, `context.TODO()` has been
added. This will be replaced with the context passed to the method when
it becomes available.
The value returned by StringGetOpts is now passed directly to the
FirstNonEmpty function, instead of being assigned to a variable
first.
The implementation of the `*AnalyzerService.GetInternalBiRPCCodec`
function has been removed from the services package. Additionally,
the AnalyzerBiRPCConnector type definition and its associated methods
have been removed.
The codec implementation has been revised to include the following
changes:
- `rpc.ServerCodec` -> `birpc.ServerCodec`;
- `rpc2.ServerCodec` -> `birpc.BirpcCodec`;
- `rpc2.Request` -> `birpc.Request`;
- `rpc2.Response` -> `birpc.Response`;
- The constructors for json and gob birpc codecs in `cenkalti/rpc`
have been replaced with ones from the `birpc/jsonrpc` library;
- The gob codec implementation has been removed in favor of the
version already implemented in the birpc external library.
The server implementation has been updated with the following changes:
- A field that represents a simple RPC server has been added to the
Server struct;
- Both the simple and bidirectional RPC servers are now initialized
inside the Server constructor, eliminating the need for nil checks;
- Usage of `net/rpc` and `cenkalti/rpc2` has been replaced with
`cgrates/birpc`;
- Additional `(Bi)RPCUnregisterName` methods have been added;
- The implementations for (bi)json/gob servers have been somewhat
simplified.
Before deleting the Call functions and using the `birpc.NewService`
method to register the methods for all cgrates components, update the
Call functions to satisfy the `birpc.ClientConnector` interface. This
way it will be a bit safer. Had to be done for SessionS though.
The `BiRPCCall` method has been removed from coreutils.go. The
`RPCCall` and `APIerRPCCall` methods are also to be removed in the
future.
Ensured that all methods for `SessionSv1` and `SessionS` have the
correct function signature with context. The same adjustments were made
for the session dispatcher methods and for the `SessionSv1Interface`.
Also removed sessionsbirpc.go and smgbirpc.go files.
Implemented the following methods to help with the registration of
methods across all subsystems:
- `NewServiceWithName`;
- `NewDispatcherService` for all dispatcher methods;
- `NewService` for the remaining methods that are already named
correctly.
Compared to the constructor from the external library, these also make
sure that the naming of the methods is consistent with our constants.
Added context to the Call methods for the mock client connectors (used
in tests).
Removed unused rpc fields from inside the following services:
- EeS
- LoaderS
- ResourceS
- RouteS
- StatS
- ThresholdS
- SessionS
- CoreS
Updated the methods implementing the logic for API methods to align
with the latest changes, ensuring consistency and correctness. The
modifications include:
- Adjusting the function signature to the new format
(ctx, args, reply).
- Prefixing names with 'V*' to indicate that they are utilized by
or registered as APIs.
- Containing the complete logic within the methods, enabling APIs
to call them and return their reply directly.
The subsystems affected by these changes are detailed as follows:
- CoreS: Additional methods were implementing utilizing the
existing ones. Though modifying them directly was possible, certain
methods (e.g., StopCPUProfiling()) were used elsewhere and not as
RPC requests.
- CDRs: Renamed V1CountCDRs to V1GetCDRsCount.
- StatS: V1GetQueueFloatMetrics, V1GetQueueStringMetrics,
V1GetStatQueue accept different arguments compared to API functions
(opted to register StatSv1 instead).
- ResourceS: Renamed V1ResourcesForEvent to V1GetResourcesForEvent
to align with API naming.
- DispatcherS: Renamed V1GetProfilesForEvent to
DispatcherSv1GetProfilesForEvent.
- For the rest, adding context to the function signature was enough.
In the unit tests, wrapping the object within a biprc.Service is now
ensured before passing it to the internal connections map under the
corresponding key.
Some tests that are covering error cases, are also checking the other
return value besides the error. That check has been removed since it
is redundant.
Revised the RPC/BiRPC clients' constructors (for use in tests)
A different approach has been chosen for the handling of ping methods
within subsystems. Instead of defining the same structure in every file,
the ping methods were added inside the Service constructor function.
Though the existing Ping methods were left as they were, they will be
removed in the future.
An additional method has been implemented to register the Ping method
from outside of the engine package.
Implemented Sleep and CapsError methods for SessionS (before they were
exclusively for bidirectional use, I believe).
A specific issue has been fixed within the CapsError SessionSv1 API
implementation, which is designed to overwrite methods that cannot be
allocated due to the threshold limit being reached. Previously, it was
deallocating when writing the response, even when a spot hadn't been
allocated in the first place (due to the cap being hit). The reason
behind this, especially why the test was passing before, still needs
to be looked into, as the problem should have occurred from before.
Implement `*SessionSv1.RegisterInternalBiJSONConn` method in apier.
All agent methods have been registered under the SessionSv1 name. For
the correct method names, the leading "V1" prefix has been trimmed
using the `birpc.NewServiceWithMethodsRename` function.
Revise the RegisterRpcParams function to populate the parameters
while relying on the `*birpc.Service` type instead. This will
automatically also deal with the validation. At the moment,
any error encountered is logged without being returned. Might
be changed in the future.
Inside the cgrRPCAction function, `mapstructure.Decode`'s output parameter
is now guaranteed to always be a pointer.
Updated go.mod and go.sum.
Fixed some typos.
Removed clone_repository variable from defaults. Now if it's not found
we will assume it's true.
In the cgrates building task we are not forcibly sourcing the /etc/profile
script anymore since it's redundant (we are using bash -lc to execute the
script, where -l will do that automatically).
Make sure we create symbolic links for all binaries we build (not just cgr-engine).
This meant the migrator task environment field could be removed as well.
Made sure failed_posts_dir exists and is empty before starting engine.
When *rerate flag is true, *refund is automatically set to true as well. Made
sure to update expected value based on that.
