Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#4294 closed defect (fixed)

[Kea 1.0.0] subnet statistics "assigned-addresses" value is incorrect

Reported by: nicolas.chaigneau Owned by: thomas
Priority: medium Milestone: Kea1.1
Component: Unclassified Version: 1.0.0
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 1
Total Hours: 63 Internal?: no

Description

When the server is started, the lease files are loaded from disk.

At this point, the statistics "assigned-addresses" for all subnets should be updated, but they are not. They are created (I think) the first time a lease is allocated, with initial value of 0, regardless of how many leases are initially allocated.

Consequently, these statistics are currently unusable.

The value should be set while loading the lease files.
Also, it would be nice to have the value set (to 0) even for subnets for which no lease is allocated.

Similarly, subnet statistics "total-addresses" could be set at the same time, currently it is not immediately available when the server starts.

Subtickets

Attachments (3)

4294Classes.svg (58.3 KB) - added by tmark 4 years ago.
4294 Classes
4294Sequence.svg (9.1 KB) - added by tmark 4 years ago.
4294 Sequence Diagram
4294_refactor.svg (19.6 KB) - added by tmark 3 years ago.
refactored classes

Download all attachments as: .zip

Change History (21)

comment:1 Changed 4 years ago by nicolas.chaigneau

Also this entails that the counter will become negative when the unaccounted for leases are reclaimed.

comment:2 in reply to: ↑ description Changed 4 years ago by nicolas.chaigneau

Replying to nicolas.chaigneau:

Similarly, subnet statistics "total-addresses" could be set at the same time, currently it is not immediately available when the server starts.

Actually it is set when the server startup is completed.
Just not when the hook library is being loaded, but we can use getPoolCapacity directly so it's not an issue.

comment:3 Changed 4 years ago by hschempf

  • Milestone changed from Kea-proposed to Kea1.1

Per team meeting Feb 11, accept 1.1

comment:4 Changed 4 years ago by tmark

  • Owner set to tmark
  • Status changed from new to assigned

Changed 4 years ago by tmark

4294 Classes

Changed 4 years ago by tmark

4294 Sequence Diagram

comment:5 Changed 4 years ago by tmark

I apologize ahead of time for the size of this change. Sue me. ;)

Approach:

There are two basic approaches to this issue. One way would be persist the statistics across reboots, the other is to provide an on-demand means to recalculate them. Per the old DBA adage of never storing what can be derived, I have opted for the latter. This avoids creating yet another form of persistence and dealing with error handling should it be corrupt or out of synch.

Solution Overview:

All lease storage back ends furnish a means to query their storage,
returning a "result set" which:

For IPv4 leases contains one row per subnet, per lease state, containing the
count of leases in that state, ordered by subnet id

and

For IPv6 leases contains: one row per subnet, per lease type, per lease state,
containing the count of leases of that type, in that state, ordered by subnet id

The base class, LeaseMgr?, then uses these abilities to fetch and then iterate
over the result sets, updating the appropriate statistics via the StatsMgr?.

After a successful configuration commit, the stats recount is initiated from
with CfgSubnet4::updateStatistics() and CfgSubnet6::updateStatistics(), which
instruct the LeaseMgr? singleton to recount.

Solution Specifics:

The following were added to LeaseMgr? (.h/.cc)

  • AddressStatsRow4 - struct that contains a single row of IPv4 lease

statistical data

  • AddressStatsQuery4 - base class for fulfilling the IPv4 statistical lease

data query. It executes the query and then provides row-by-row access to
the result sets.

  • AddressStatsRow6 - struct that contains a single row of IPv6 lease

statistical data

  • AddressStatsQuery6 - base class for fulfilling the IPv6 statistical lease

data query. It executes the query and then provides row-by-row access to
the result sets.

  • LeaseMgr::recountAddressStats4() - method that recalculates per-subnet

and global stats for IPv4 leases

  • LeaseMgr::startAddressStatsQuery4() - virtual method which creates

then executes the IPv4 lease stats query

  • LeaseMgr::recountAddressStats6() - method that recalculates per-subnet

and global stats for IPv6 leases

  • LeaseMgr::startAddressStatsQuery6() - virtual method which creates

then executes the IPv6 lease stats query

Finally, derivations of AddressStatsQuery4/6 and implementations of
startAddressStats4/6 were added to the Memfile, MySQL, and PostgreSQL
lease managers. Support for Cassandra has not yet been done.

In the case of Memfile, an additional non-unique index per subnet IDS was added
to both Lease4Storage and Lease6Storage. This was done such that the leases
could be iterated over in order by subnet_id, rather than attempting to process them in
random order. Preliminary testing with a CSV persisted lease file of 1.1M IPv4
leases showed the recount process added less then 1/2 second to reconfiguration. In
comparison it took nearly 20 seconds to load the file itself. What remains to be seen
is if the addition of the index has any significant impact to performance in general.

In the case of MySQL and PostgreSQl, the solution is good bit simpler. The result
sets are generated by the following SQL statements (identical statements for both
RDBMs):

SELECT subnet_id, state, count(state) as state_count 
FROM lease4 GROUP BY subnet_id, state ORDER BY subnet_id
SELECT subnet_id, lease_type, state, count(state) as state_count
FROM lease6 GROUP BY subnet_id, lease_type, state
ORDER BY subnet_id

I've included a class diagram which shows the new classes (in blue) and
the modified classes (in yellow), followed by a sequence diagram that
shows the chain of events for recounting the IPv6 lease statistics for
a PostgreSQL lease manager. The sequence is essentially for the other
lease managers and v4 versus v6.

Class Diagram:

4294 Classes

IPv6 PostgreSQL Sequence Diagram
4294 Sequence Diagram

ChangeLog?:

11xx.   [bug]       tmark
    Lease statistics are now recalculated during server startup and
    after each successful reconfiguration for Memfile, MySQL, and PostgreSQL
    back ends.  This addressses issues caused by accumulated values being lost
    across restarts and reconfigurations making rendering values incorrect.
    (Trac #4294 git TBD)
Last edited 4 years ago by tmark (previous) (diff)

comment:6 Changed 4 years ago by tmark

  • Add Hours to Ticket changed from 0 to 40
  • Owner changed from tmark to Unassigned
  • Status changed from assigned to reviewing
  • Total Hours changed from 0 to 40

comment:7 Changed 4 years ago by tomek

  • Owner changed from Unassigned to tomek

comment:8 follow-up: Changed 3 years ago by tomek

  • Add Hours to Ticket changed from 40 to 8
  • Total Hours changed from 40 to 48

First of all, I'm very impressed with the scope of changes you made
to solve this issue. You're not dodging anything here. You did
what had to be done to get to the bottom of the problem. I take
my hat off to you, sir!

Ok, on to more mundane stuff. The code didn't compile for me on
Ubuntu 16.04. The getNextRow() method has row parameter that was not
used in the actual implementation, causing a warning about unused
parameter. That issue appeared twice in the code. It was an easy
fix. Please pull.

I have several comments. Some of them may or may not require rather
significant changes. Given the time left we have before beta, we
need to be flexible with this. The code as it is now seems to be
working. It's ok if you prefer to push most of this work to a separate
ticket for past beta.

lease.cc|h
Please revert the change you did for lease states. Even though the
constants may look like enums, they're really bitfields, with the
default having all states cleared. The next value would be 4, not

  1. One thing to keep in mind is that if we ever going to implement

failover, there will be another bit set here that works completely
independent to those existing. There are 8 states a lease can go
through in a failover. You do not want to enumerate full permutation
of those together with our regular lease states.

@todo in recountAddressStats4(). The todo asks whether it would be OK
to clear them with the rest. It's fine to clear them with the rest,
but we should think how the code works and whether it covers the
following 3 cases properly:

  1. we had X subnets, now we have X+1
  2. we had X subnets, now we have X, but with different subnet-ids
  3. we had X subnets, now we have X-1

By properly I mean we need to remove statistics for subnets that
are no longer useful. The separate was an attempt to keep the
values mostly sane after reconfigure. But with the code you added,
we don't need to worry about that and simply recalculate everything.

lease_mgr.h
Can you use one class for both statistical data? I know it's tempting
to have separate classes for everything, but one day we will face
the task of running Kea on embedded devices. Kea is already big
and with each new code added it tends to get bigger and bigger.

There's another reason to go with the lease_type_ being being
specified, even if the only allowed lease type in v4 is
Lease::TYPE_V4. The RFCs that document allocation of port-sets in v4
are published and we already received inquiries from prospective
users whether we're planning to support them. So in the not so
distant future we may see two types of leases in V4: full addresses as
we have them now and address+port-set type of leases, which would
clearly be a very different type (as they could have overlapping
addresses).

Third reason why this should be a single class is that we would have
a clear understanding - either a backend supports statistics update
or not. We know that the good folks who contributed Cassandra code
are not interested in IPv4, so with the current code they may simply
choose to skip implementing v4 statistics. If it's all bundled in
one class, they will either completely support it or not support it at
all.

Fourth and probably the most compelling reason is that there is a lot
of duplication between v4 and v6 instances.

I admit that each of those arguments alone is not sufficient, but
together they are somewhat appealing. If my arguments haven't
convinced you or if this change requires too much work or it would
impact beta, I can accept it as is. But then we'll likely need a
refactoring ticket. Unless you convince me that all of the above is
rubbish, that is :) If you don't want to use a joint class for both,
maybe at least you could merge AddressStatsRow4 and AddressStatsRow6?
There's a Lease::TYPE_V4 defined for a reason.

Also, the class should not have 'address' in its name. It should be
LeaseStatsQuery6 or better yet StatsQuery6, not
AddressStatsQuery6. That name is better for at least three
reasons. First, it operates on leases, not addresses. Second, we have
addresses and prefixes in v6. Third, we may soon have addresses and
addresses+port-sets (which are really parts of the address) in v4.
So it's better to not explicitly have "address" in its name.

The comment for recountAddressStats4() says AddressStats4Qry, should
be AddressStatsQuery4 (the same mistake is done for
recountAddressStats6).

memfile_lease_storage.h

I was thinking whether adding another index to the container is the
right way to go. Since we need to walk through all of the leases
anyway, what's the benefit of having an index for it? Also, the
statistics are updated infrequently and in very controlled moments, so
it's ok if this introduces a bit of a delay. On the other hand, there's small
degradation of performance during normal operation that affects all
insert and update operations, that we typically do 1000s times each
second. But I admit I don't have any empirical data to back my fears.
Unless you feel it's a serious problem, let's keep it as it is for
now, but we should ask Wlodek to measure performance of memfile before
and after this change is merged. Hopefully the impact is negligible
and we can keep the code as is.

memfile_lease_mgr.h
AddressStatesQuery4Ptr comment: it's => its

memfile_lease_mgr.cc

MemfileAddressStatsQuery4 logic of calling start() and setting
next_pos_ make it inherently stateful and thus multi-threaded
unfriendly. That's fine for now, but I think we need to think about
how we want to do statistics in a bit broader context, especially once
we start taking advantage of multiple cores. Depending on the answer,
the architecture you proposed may or may not be a problem. But it's
still far off in the future and we don't have any concrete designs or
achitectures, so no need to change anything right now.

In the future, I think we should drop the approach of returning an
object and then iterating over the rows in favor of returning the
whole result is one call. So start() and getNextRow() would be
replaced with a single getStats() call.

mysql_lease_mgr.cc
MySqlAddressStatsQuery4 - see my comments above about having Address in the
name of this and related classes. Also, the bind_(3) requires a
comment in the constructor.

Something is wrong with the comment for checkError in line 1452. Did
you forget to add @throw before DbOperation??

pgsql_lease_mgr.cc
containining contain => containining

srv_config.cc
Regarding your @todo in updateStatistics(), the whole libdhcpsrv is
linked in D2. I think the reason is that there's a
DControllerBase::getVersion() method that for some reason prints out
versions of the backands. Which doesn't make sense at all.


I have built the code with MySQL and PostgreSQL support and ran all
tests on Ubuntu 16.04 x64 and they passed.

The proposed ChangeLog? looks good, except one extra s in addresses.

This concludes the part of the review that deals with the code.
The unit-tests review will follow.

comment:9 follow-up: Changed 3 years ago by tomek

  • Add Hours to Ticket changed from 8 to 1
  • Owner changed from tomek to thomas
  • Total Hours changed from 48 to 49

Here are the remaining comments for unit-tests. Just a small bunch of nitpicks.

alloc_engine_utils.cc
Can you add a short comment why you moved factory_.create() in
AllocEngine?{4,6}Test constructor?

generic_lease_mgr_unittest.h
checkAddressStats - comment need update. Something isn't right here:
"Fails any the stat is not found"

generic_leass_mgr_unittest.cc
GenericLeaseMgrTest::testRecountAddressStats4() - Looks like there
are only 2 subnets, so the comment in line 2383 could use a bit of
tweaking. The same is true for v6 code.

dhcpsrv_messages.mes
TOMS_UTILITY_MESSAGE - We should probably rename it to
TOMS_AWESOME_UTILITY_MESSAGE. And, if you think the world is not
ready for such awesomeness yet, we may delete it. For now, that is.

Changed 3 years ago by tmark

refactored classes

comment:10 in reply to: ↑ 8 Changed 3 years ago by tmark

Replying to tomek:

First of all, I'm very impressed with the scope of changes you made
to solve this issue. You're not dodging anything here. You did
what had to be done to get to the bottom of the problem. I take
my hat off to you, sir!

Why thank you.

Ok, on to more mundane stuff. The code didn't compile for me on
Ubuntu 16.04. The getNextRow() method has row parameter that was not
used in the actual implementation, causing a warning about unused
parameter. That issue appeared twice in the code. It was an easy
fix. Please pull.

Oops. I don't know how I missed that. Thanks.

I have several comments. Some of them may or may not require rather
significant changes. Given the time left we have before beta, we
need to be flexible with this. The code as it is now seems to be
working. It's ok if you prefer to push most of this work to a separate
ticket for past beta.

lease.cc|h
Please revert the change you did for lease states. Even though the
constants may look like enums, they're really bitfields, with the
default having all states cleared. The next value would be 4, not

One thing to keep in mind is that if we ever going to implement

failover, there will be another bit set here that works completely
independent to those existing. There are 8 states a lease can go
through in a failover. You do not want to enumerate full permutation
of those together with our regular lease states.

Sigh. I asked Marcin about that specifically and he had no issues with it.
I have reverted it.

@todo in recountAddressStats4(). The todo asks whether it would be OK
to clear them with the rest. It's fine to clear them with the rest,
but we should think how the code works and whether it covers the
following 3 cases properly:

we had X subnets, now we have X+1
we had X subnets, now we have X, but with different subnet-ids
we had X subnets, now we have X-1

By properly I mean we need to remove statistics for subnets that
are no longer useful. The separate was an attempt to keep the
values mostly sane after reconfigure. But with the code you added,
we don't need to worry about that and simply recalculate everything.

The recalculation relies on the fact that CfgMgr::commit() removes the
statistics for the current configuration prior to committing the configuration:

" First we need to remove statistics. The new configuration can have fewer
subnets. Also, it may change subnet-ids. So we need to remove them all
and add it back."

so it should work correctly. If and when we implement a recalculate via
the control channel, that code should also do a remove first.

lease_mgr.h
Can you use one class for both statistical data? I know it's tempting
to have separate classes for everything, but one day we will face
the task of running Kea on embedded devices. Kea is already big
and with each new code added it tends to get bigger and bigger.

There's another reason to go with the lease_type_ being being
specified, even if the only allowed lease type in v4 is
Lease::TYPE_V4. The RFCs that document allocation of port-sets in v4
are published and we already received inquiries from prospective
users whether we're planning to support them. So in the not so
distant future we may see two types of leases in V4: full addresses as
we have them now and address+port-set type of leases, which would
clearly be a very different type (as they could have overlapping
addresses).

Third reason why this should be a single class is that we would have
a clear understanding - either a backend supports statistics update
or not. We know that the good folks who contributed Cassandra code
are not interested in IPv4, so with the current code they may simply
choose to skip implementing v4 statistics. If it's all bundled in
one class, they will either completely support it or not support it at
all.

Fourth and probably the most compelling reason is that there is a lot
of duplication between v4 and v6 instances.

I admit that each of those arguments alone is not sufficient, but
together they are somewhat appealing. If my arguments haven't
convinced you or if this change requires too much work or it would
impact beta, I can accept it as is. But then we'll likely need a
refactoring ticket. Unless you convince me that all of the above is
rubbish, that is :) If you don't want to use a joint class for both,
maybe at least you could merge AddressStatsRow4 and AddressStatsRow6?
There's a Lease::TYPE_V4 defined for a reason.

I toyed with this quite a bit and in the end, decided against refactoring it until
it had undergone a review first. I was also a bit reluctant to pass around a lease type for V4 when it was pointless. However, if it may come to pass that we have multiple lease types for V4 that's fine. I have refactored it quite a bit, enough that I think you'll be satisfied. Memfile still implements two separate classes but they must work with different storage types. I also changed the names from Address to Lease. I kept the notion of Lease in the name as these classes are intended strictly for dealing with lease statistics. Updating other statistics would mean different queries, result content et al.

I've included an updated diagram for your revieiwng pleasure:

refactored classes

Also, the class should not have 'address' in its name. It should be
LeaseStatsQuery6 or better yet StatsQuery6, not
AddressStatsQuery6. That name is better for at least three
reasons. First, it operates on leases, not addresses. Second, we have
addresses and prefixes in v6. Third, we may soon have addresses and
addresses+port-sets (which are really parts of the address) in v4.
So it's better to not explicitly have "address" in its name.

See above

The comment for recountAddressStats4() says AddressStats4Qry, should
be AddressStatsQuery4 (the same mistake is done for
recountAddressStats6).

Done

memfile_lease_storage.h

I was thinking whether adding another index to the container is the
right way to go. Since we need to walk through all of the leases
anyway, what's the benefit of having an index for it? Also, the
statistics are updated infrequently and in very controlled moments, so
it's ok if this introduces a bit of a delay. On the other hand, there's small
degradation of performance during normal operation that affects all
insert and update operations, that we typically do 1000s times each
second. But I admit I don't have any empirical data to back my fears.
Unless you feel it's a serious problem, let's keep it as it is for
now, but we should ask Wlodek to measure performance of memfile before
and after this change is merged. Hopefully the impact is negligible
and we can keep the code as is.

The primary reason for the index is so the accumulating code can iterator over them
in order by subnet, otherwise the order would be random we would have to either:

A: Use a map of accumulators keyed but subnet id to construct the result set in
MemfileLeaseStatsQuery4/6::start(). Since subnet ids are numerical the map accesses
would be fairly quick. Basically we'd change MemfileLeaseStatsQuery::rows_ from
this vector<LeaseStatsRow?> to this map<SubnetID, LeaseStatsRow?>.

B: Rather than use the LeaseStatsQuery? model which constructs a result set, override
LeaeMgr::recountLeaseStats4() and 6() in Memfile_LeaseMgr and accumulate directly into
the stats via StatsMgr?. This could be fairly painful as each stat access is by a rather
long string name and you would have one access per lease.

As per our jabber session, the performance impact is unknown and we should check it. If
it turns out to be negligible, which I suspect it will be, great. If not, I think I would
opt for solution A above.

memfile_lease_mgr.h
AddressStatesQuery4Ptr comment: it's => its

Oops.

memfile_lease_mgr.cc

MemfileAddressStatsQuery4 logic of calling start() and setting
next_pos_ make it inherently stateful and thus multi-threaded
unfriendly. That's fine for now, but I think we need to think about
how we want to do statistics in a bit broader context, especially once
we start taking advantage of multiple cores. Depending on the answer,
the architecture you proposed may or may not be a problem. But it's
still far off in the future and we don't have any concrete designs or
achitectures, so no need to change anything right now.

In the future, I think we should drop the approach of returning an
object and then iterating over the rows in favor of returning the
whole result is one call. So start() and getNextRow() would be
replaced with a single getStats() call.

Ok, I disagree somewhat here. The LeaseStatsQuery? implementations already
contain a "result set" and they are basically the thing returned. In the case
of Memfile it is stored internally as a vector<>, in the case of MySql? it is
the bind array, and in the case of PgSql? it is within the PgSqlResult?. What your
suggesting would mean in the case of the latter two would require replicating
the data from the RDBMs storage into a vector<>. In other words, we would replicate
all of the data from one form of storage into another needlessly.

Secondly, LeaseStatsQuery? instances are not singletons, and each instance should
be self-contained. It ought to be possible for there to be more than one instance
at a time, though I'm having a hard time imagining why we would want more than one
instance against the same storage/db connection at a time. Certainly, it is safe
for both Memfile and PostgreSQL. I'm less certain about MySQL as the client side
resources allocated for the execution of a prepared statement are associated with
that statement. I do not think you can maintain more than one result set against the
same statement. But again, I don't see where a single DB connection would want to
run the same query more than once concurrently. If we have mulitple threads accessing
storage they should do so using their own connections.

For now I am leaving this unchanged.

mysql_lease_mgr.cc
MySqlAddressStatsQuery4 - see my comments above about having Address in the
name of this and related classes. Also, the bind_(3) requires a
comment in the constructor.

Done.

Something is wrong with the comment for checkError in line 1452. Did
you forget to add @throw before DbOperation???

Actually I have removed checkError.

pgsql_lease_mgr.cc
containining contain => containining

Must have gotten deleted in the refactor.

srv_config.cc
Regarding your @todo in updateStatistics(), the whole libdhcpsrv is
linked in D2. I think the reason is that there's a
DControllerBase::getVersion() method that for some reason prints out
versions of the backands. Which doesn't make sense at all.

Isn't there a ticket for this already?

I will address your test comments in that comment.

comment:11 in reply to: ↑ 9 Changed 3 years ago by tmark

  • Add Hours to Ticket changed from 1 to 9
  • Owner changed from thomas to tomek
  • Total Hours changed from 49 to 58

Replying to tomek:

Here are the remaining comments for unit-tests. Just a small bunch of nitpicks.

alloc_engine_utils.cc
Can you add a short comment why you moved factory_.create() in
AllocEngine?{4,6}Test constructor?

Done.

generic_lease_mgr_unittest.h
checkAddressStats - comment need update. Something isn't right here:
"Fails any the stat is not found"

Never type distracted! Fixed.

generic_leass_mgr_unittest.cc
GenericLeaseMgrTest::testRecountAddressStats4() - Looks like there
are only 2 subnets, so the comment in line 2383 could use a bit of
tweaking. The same is true for v6 code.

Gee, you're picky. ;)

dhcpsrv_messages.mes
TOMS_UTILITY_MESSAGE - We should probably rename it to
TOMS_AWESOME_UTILITY_MESSAGE. And, if you think the world is not
ready for such awesomeness yet, we may delete it. For now, that is.

I LIKE IT! I forgot about this.

Ticket is back to you for re-review.

comment:12 Changed 3 years ago by tomek

  • Add Hours to Ticket changed from 9 to 3
  • Owner changed from tomek to thomas
  • Total Hours changed from 58 to 61

Ok, this's gonna be a quick one. Thanks a lot for the code update. The code
looks much better now. I like the new class diagram.

The code didn't compile, because of unused variable in pgsql_lease_mgr.cc.
Trivial fix pushed.

cfg_subnets4.cc
Removed commented out code in line 272.

memfile_lease_mgr.cc
Fixed typo: storgae => storage

I have reviewed your changes and they look good.

I ran all unit-tests (with MySQL and PostgreSQL enabled) on Ubuntu
16.04 x64 and they all passed.

The code is ready for merge. Please merge the code, but don't close this
ticket yet. It was reported by Nicolas, so we need to check up with
him if the fix solves the issue he encountered.

comment:13 Changed 3 years ago by tmark

  • Add Hours to Ticket changed from 3 to 1
  • Owner changed from thomas to tomek
  • Total Hours changed from 61 to 62

Per our jabber discussion I have modified the Memfile implementation to use the existing per-address indexes and have removed the per-subnet_id indexes. Walking the leases in order by address should naturually result in grouping them by subnet_id allowing us to accumluate the state counts as before. Although the result sets will not be in order by subnet id, that actually doesn't matter and it removes the potential performance impact of the additional index on lease insertion/retrieval/removal.

Please review these changes.

comment:14 Changed 3 years ago by tomek

  • Owner changed from tomek to thomas
  • Total Hours changed from 62 to 63

The changes are good. Please merge.

comment:15 Changed 3 years ago by tmark

Changes merged with git 0abdcf15f85861ffcb67d50fa4ce3965d25e4a9f
ChangeLog? entry 1156 added.

Ticket is complete but we're holding open until it can be verified by the reporter, Nicolas Chaigneau.

comment:16 Changed 3 years ago by nicolas.chaigneau

To be honest I've not read all. This is more complicated than I imagined it would :)
I'll test it when 1.1 beta is available.

Thanks for working on this!

comment:17 Changed 3 years ago by tomek

  • Resolution set to fixed
  • Status changed from reviewing to closed

To the best of our knowledge the solution developed addresses the original problem of statistics getting out of sync. The improved code has now been released in Kea 1.1.0-beta, so we're closing this ticket.

Nicolas, if you later discover that this change doesn't fix the problem, feel free to either reopen this ticket or create a new one.

Thanks for using Kea.

comment:18 Changed 3 years ago by nicolas.chaigneau

I've tested Kea 1.1.0-beta, statistics are now fully functional.
Thanks again for the hard work :)

I have another issue, but not directly related, so I'll move the discussion to the mailing list.

Note: See TracTickets for help on using tickets.