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.

Sign up for GitHub

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

Proof of concept for MaxDispatches implementation #2598

Closed
wants to merge 5 commits into from
Closed

Proof of concept for MaxDispatches implementation #2598

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6aadb09
POC for MaxDispatches
bentoi Aug 1, 2024
89ed50d
Fixes
bentoi Aug 2, 2024
fdcd7d8
Merge remote-tracking branch 'origin/main' into maxdispatch
bentoi Aug 2, 2024
e44ff3d
Added max connections
bentoi Aug 2, 2024
2bd882b
Merge remote-tracking branch 'origin/main' into maxdispatch
bentoi Aug 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config/PropertyNames.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ generated from the section label.
<suffix name="EnableIdleCheck" default="1" />
<suffix name="IdleTimeout" default="60" />
<suffix name="InactivityTimeout" default="300" />
<suffix name="MaxDispatches" default="0" />
</class>

<class name="threadpool" prefix-only="true">
Expand Down
143 changes: 94 additions & 49 deletions cpp/src/Ice/PropertyNames.cpp
View file
Open in desktop

Large diffs are not rendered by default.

85 changes: 45 additions & 40 deletions cpp/src/Ice/PropertyNames.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright (c) ZeroC, Inc. All rights reserved.

// Generated by makeprops.py from PropertyNames.xml, Tue Jul 2 13:49:12 2024
// Generated by makeprops.py from PropertyNames.xml, Thu Aug 1 10:41:04 2024

// IMPORTANT: Do not edit this file -- any edits made here will be lost!

Expand All @@ -12,53 +12,58 @@
namespace IceInternal
{

struct Property
struct Property
{
const char* pattern;
bool usesRegex;
const char* defaultValue;
bool deprecated;

Property(const char* n, bool r, const char* dv, bool d) :
pattern(n),
usesRegex(r),
defaultValue(dv),
deprecated(d)
{
const char* pattern;
bool usesRegex;
const char* defaultValue;
bool deprecated;
}

Property(const char* n, bool r, const char* dv, bool d)
: pattern(n),
usesRegex(r),
defaultValue(dv),
deprecated(d)
{
}
Property() = delete;
};

Property() = delete;
};
struct PropertyArray
{
const Property* properties;
const int length;

struct PropertyArray
PropertyArray(const Property* p, size_t len) :
properties(p),
length(static_cast<int>(len))
{
const Property* properties;
const int length;
}
};

PropertyArray(const Property* p, size_t len) : properties(p), length(static_cast<int>(len)) {}
};
class PropertyNames
{
public:

class PropertyNames
{
public:
static const PropertyArray IceProps;
static const PropertyArray IceMXProps;
static const PropertyArray IceDiscoveryProps;
static const PropertyArray IceLocatorDiscoveryProps;
static const PropertyArray IceBoxProps;
static const PropertyArray IceBoxAdminProps;
static const PropertyArray IceBridgeProps;
static const PropertyArray IceGridAdminProps;
static const PropertyArray IceGridProps;
static const PropertyArray IceSSLProps;
static const PropertyArray IceStormAdminProps;
static const PropertyArray IceBTProps;
static const PropertyArray Glacier2Props;
static const PropertyArray Glacier2CryptPermissionsVerifierProps;
static const PropertyArray IceProps;
static const PropertyArray IceMXProps;
static const PropertyArray IceDiscoveryProps;
static const PropertyArray IceLocatorDiscoveryProps;
static const PropertyArray IceBoxProps;
static const PropertyArray IceBoxAdminProps;
static const PropertyArray IceBridgeProps;
static const PropertyArray IceGridAdminProps;
static const PropertyArray IceGridProps;
static const PropertyArray IceSSLProps;
static const PropertyArray IceStormAdminProps;
static const PropertyArray IceBTProps;
static const PropertyArray Glacier2Props;
static const PropertyArray Glacier2CryptPermissionsVerifierProps;

static const PropertyArray validProps[];
static const char* clPropNames[];
};
static const PropertyArray validProps[];
static const char* clPropNames[];
};

}

Expand Down
35 changes: 29 additions & 6 deletions csharp/src/Ice/ConnectionI.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,12 @@ public override void message(ThreadPoolCurrent current)
}
}

if (_maxDispatches > 0 && _dispatchCount == _maxDispatches)
Copy link
Member

Choose a reason for hiding this comment

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

Should this be _dispatchCount >= _maxDispatches, since _dispatchCount can be incremented by more than 1 when we received a batch request?

{
// Only read from the connection if max dispatches isn't reached.
newOp &= ~SocketOperation.Read;
bentoi marked this conversation as resolved.
Show resolved Hide resolved
}

// If the connection is not closed yet, we can update the thread pool selector to wait for
// readiness of read, write or both operations.
if (_state < StateClosed)
Expand Down Expand Up @@ -1376,6 +1382,7 @@ internal ConnectionI(
_closeTimeout = options.closeTimeout; // not used for datagram connections
// suppress inactivity timeout for datagram connections
_inactivityTimeout = endpoint.datagram() ? TimeSpan.Zero : options.inactivityTimeout;
_maxDispatches = options.maxDispatches;
_removeFromFactory = removeFromFactory;
_warn = initData.properties.getIcePropertyAsInt("Ice.Warn.Connections") > 0;
_warnUdp = initData.properties.getIcePropertyAsInt("Ice.Warn.Datagrams") > 0;
Expand Down Expand Up @@ -1647,31 +1654,40 @@ private void setState(int state)
case StateActive:
{
//
// Can only switch from holding or not validated to
// active.
// Can only switch to active from holding or not validated.
//
if (_state != StateHolding && _state != StateNotValidated)
{
return;
}

if (_maxDispatches > 0 && _dispatchCount == _maxDispatches)
bentoi marked this conversation as resolved.
Show resolved Hide resolved
{
// Don't resume reading if maxDispatches is reached.
return;
}

_threadPool.register(this, SocketOperation.Read);
break;
}

case StateHolding:
{
//
// Can only switch from active or not validated to
// holding.
// Can only switch to holding from active or not validated.
bentoi marked this conversation as resolved.
Show resolved Hide resolved
//
if (_state != StateActive && _state != StateNotValidated)
{
return;
}
if (_state == StateActive)

if (_maxDispatches > 0 && _dispatchCount == _maxDispatches)
bentoi marked this conversation as resolved.
Show resolved Hide resolved
{
_threadPool.unregister(this, SocketOperation.Read);
// Reads are already disabled if maxDispatches is reached.
return;
}

_threadPool.unregister(this, SocketOperation.Read);
bentoi marked this conversation as resolved.
Show resolved Hide resolved
break;
}

Expand Down Expand Up @@ -2516,6 +2532,12 @@ private void sendResponse(OutgoingResponse response, bool isTwoWay, byte compres
sendMessage(new OutgoingMessage(response.outputStream, compress > 0, adopt: true));
}

if (_maxDispatches > 0 && _state != StateHolding && _dispatchCount == _maxDispatches)
Copy link
Member

Choose a reason for hiding this comment

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

I am not following here. The _maxDispatches is "active" only state = StateHolding? Should it be only when state = StateActive?

Copy link
Member Author

Choose a reason for hiding this comment

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

Did you expect _state == StateActive instead? It's pretty much the same.

{
// Resume reads for this connection.
_threadPool.update(this, SocketOperation.None, SocketOperation.Read);
}

--_dispatchCount;

if (_state == StateClosing && _upcallCount == 0)
Expand Down Expand Up @@ -2896,6 +2918,7 @@ internal void completed(LocalException ex)

// The number of outstanding dispatches. Maintained only while state is StateActive or StateHolding.
private int _dispatchCount;
private readonly int _maxDispatches;

private int _state; // The current state.
private bool _shutdownInitiated;
Expand Down
3 changes: 2 additions & 1 deletion csharp/src/Ice/ConnectionOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ internal sealed record class ConnectionOptions(
TimeSpan closeTimeout,
TimeSpan idleTimeout,
bool enableIdleCheck,
TimeSpan inactivityTimeout);
TimeSpan inactivityTimeout,
int maxDispatches);
13 changes: 8 additions & 5 deletions csharp/src/Ice/Internal/Instance.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -706,11 +706,10 @@ internal void initialize(Ice.Communicator communicator, Ice.InitializationData i
closeTimeout: TimeSpan.FromSeconds(properties.getIcePropertyAsInt("Ice.Connection.CloseTimeout")),
idleTimeout: TimeSpan.FromSeconds(properties.getIcePropertyAsInt("Ice.Connection.IdleTimeout")),
enableIdleCheck: properties.getIcePropertyAsInt("Ice.Connection.EnableIdleCheck") > 0,
inactivityTimeout: TimeSpan.FromSeconds(properties.getIcePropertyAsInt("Ice.Connection.InactivityTimeout")));

inactivityTimeout: TimeSpan.FromSeconds(properties.getIcePropertyAsInt("Ice.Connection.InactivityTimeout")),
maxDispatches: properties.getIcePropertyAsInt("Ice.Connection.MaxDispatches"));
{
int num =
_initData.properties.getIcePropertyAsInt("Ice.MessageSizeMax");
int num = _initData.properties.getIcePropertyAsInt("Ice.MessageSizeMax");
if (num < 1 || num > 0x7fffffff / 1024)
{
_messageSizeMax = 0x7fffffff;
Expand Down Expand Up @@ -1436,7 +1435,11 @@ internal ConnectionOptions serverConnectionOptions(string adapterName)

inactivityTimeout: TimeSpan.FromSeconds(properties.getPropertyAsIntWithDefault(
$"{adapterName}.Connection.InactivityTimeout",
(int)clientConnectionOptions.inactivityTimeout.TotalSeconds)));
(int)clientConnectionOptions.inactivityTimeout.TotalSeconds)),

maxDispatches: properties.getPropertyAsIntWithDefault(
$"{adapterName}.Connection.MaxDispatches",
(int)clientConnectionOptions.maxDispatches));
bentoi marked this conversation as resolved.
Show resolved Hide resolved
}
else
{
Expand Down
Loading
Loading