Opened 7 years ago

Closed 7 years ago

#2327 closed defect (fixed)

Allocation engine v6: expiration

Reported by: tomek Owned by: tomek
Priority: medium Milestone: Sprint-DHCP-20130103
Component: dhcp6 Version:
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: 0
Total Hours: 0 Internal?: no

Description

This ticket covers v6 address expiration. See #2323 for v4 counterpart.

Subtickets

Change History (9)

comment:1 Changed 7 years ago by stephen

  • Milestone changed from Sprint-DHCP-20121115 to DHCP 2012

comment:2 Changed 7 years ago by stephen

  • Milestone changed from DHCP Outstanding Tasks to Sprint-DHCP-20130103

comment:3 Changed 7 years ago by tomek

  • Owner set to tomek
  • Status changed from new to accepted

comment:4 Changed 7 years ago by tomek

  • Owner changed from tomek to UnAssigned
  • Status changed from accepted to reviewing

Support for recycling reused leases has been added. Please review.

comment:5 Changed 7 years ago by stephen

  • Owner changed from UnAssigned to stephen

comment:6 Changed 7 years ago by stephen

  • Owner changed from stephen to tomek

src/lib/dhcpsrv/alloc_engine.cc
Suggest a comment at line 210 along the lines of "Hint is in the pool but is not available - search the pool until we find a free address of one for which the lease has expired"·

Line 219/220: strictly speaking this comment should be after the following "if" statement.

Line 253: just a note for the future, when we go multiprocess, there could be a race condition here - an expired lease is chosen, then some other process renews it. However, we don't have to worry about that now.

src/lib/dhcpsrv/tests/alloc_engine_unittest.cc
Can we move "detailCompareLease6()" into a separate file and share it with mysql_lease_mgr_unittests.cc?

ChangeLog is OK.

comment:7 Changed 7 years ago by tomek

  • Owner changed from tomek to stephen

I've added an new files called test_utils.{cc|h}. The idea here is that it will contain more methods that can be shared between tests. As there could be potentially tens of small utility functions, I think it should be one mix-and-match file, rather then lots of small files. They are all linked together anyway.

Added comments as requested. In particular commented on drawback on current approach to expiration, i.e. we can't do reference counting for subnets. That is not necessarily a bad thing - it is just a tradeoff. We work better in the average case, but we will be very slow when totally out of available addresses.

Personally, I think this area will be improved many times throughout the lifetime of this software. One idea briefly discussed was to implement dynamic switch between allocation policies depending on current conditions (e.g. running over 90% allocation? give up on random allocator and switch to iterative).

Fixes pushed. Ok to merge?

comment:8 Changed 7 years ago by stephen

  • Owner changed from stephen to tomek

Reviewed commit 62a23854f619349d319d02c3a385d9bc55442d5e.

Personally, I think this area will be improved many times throughout the lifetime of this software. One idea briefly discussed was to implement dynamic switch between allocation policies depending on current conditions (e.g. running over 90% allocation? give up on random allocator and switch to iterative).

I think that will be highly likely.

All OK, please merge.

comment:9 Changed 7 years ago by tomek

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

Merged. Thanks for your review.

Note: See TracTickets for help on using tickets.