Opened 5 years ago

Closed 5 years ago

#3685 closed task (fixed)

LFC: Update the Kea LFC design with the most recent comments on the mail list

Reported by: marcin Owned by: sar
Priority: low Milestone: Kea0.9.1beta
Component: documentation Version: git
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

There has been some comments on the kea-dev mailing list with respect to the Lease File Cleanup design after the design ticket has been closed. The specific threads are:

The LFC Design document should be updated accordingly.

Subtickets

Change History (9)

comment:1 Changed 5 years ago by tomek

  • Milestone changed from Kea-proposed to Kea0.9.1
  • Priority changed from medium to low

comment:2 Changed 5 years ago by marcin

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

comment:3 Changed 5 years ago by marcin

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

I updated the design document according to the comments in the aforementioned list threads.

I also removed the requirement (from the requirements doc) for preserving unparsable data during LFC. This was a result of the decision that the kea-lfc should read all leases from the input file before dumping the cleaned up information to the output file. This approach results in more efficient clean up of the lease file, but it makes it troublesome to preserve the unparsable information, which is considered less important comparing to the importance of the efficient cleanup.

Please review.

comment:4 follow-up: Changed 5 years ago by sar

1) In the terminology section the document states that source files are not modified by LFC. This is true if "source files" means the current lease file. It isn't true if it includes the previous lease file or the copy of the current lease file (<lease-filename>.2 and <lease-filename>.1). Both of those will be deleted and the previous lease file will be replaced.

2) In the description of the file names for "Lease File" the last line of the description states that this file and the previous lease file contain the full lease information. The full lease information also requires the lease file copy in the case that the server has moved Lease File to Lease File Copy and re-created Lease File but has not yet run LFC.

3) Again in the description of the file names this time for LFC Finish File. The description states that moving the output file to this name is the final operation performed by kea-lfc. It is the final step of the lease processing, however after this the kea-lfc process will continue by removing the working files and then move this file to Previous lease file.

4) In paragraph 2 of LFC process overview the documental has "Regardless if the Lease File copy exists or not," This should be "Regardless if the Lease File copy existed or not," as by this point we have renamed the current Lease File to the Lease File Copy.

5) Same paragraph has "it replaces moves the" which should be one or the other, "it replaces the".

6) In the section on kea-lfc. The code currently uses -p for the pid file and -x for the previous lease file.

7) In the section on preventing concurrent runs - having the server process restart and then kill the "LFC" process based on its PID is a bit dangerous. Because the server has been restarted it can't be sure that the current process with that PID is actually the LFC process and not some other process that has been started in the mean time. While we may go with choosing an specific option for now we may eventually want to add a configuration option to allow the administrator to decide what they want to happen - 1) kill the process, 2) wait for the process to finish (no matter how long), 3) wait for some duration and then delete the PID and start the LFC, 4) log an error and let the administrator sort it out.

comment:5 Changed 5 years ago by stephen

  • Owner changed from UnAssigned to marcin

comment:6 Changed 5 years ago by marcin

  • Owner changed from marcin to sar

comment:7 in reply to: ↑ 4 Changed 5 years ago by marcin

Replying to sar:

1) In the terminology section the document states that source files are not modified by LFC. This is true if "source files" means the current lease file. It isn't true if it includes the previous lease file or the copy of the current lease file (<lease-filename>.2 and <lease-filename>.1). Both of those will be deleted and the previous lease file will be replaced.

2) In the description of the file names for "Lease File" the last line of the description states that this file and the previous lease file contain the full lease information. The full lease information also requires the lease file copy in the case that the server has moved Lease File to Lease File Copy and re-created Lease File but has not yet run LFC.

3) Again in the description of the file names this time for LFC Finish File. The description states that moving the output file to this name is the final operation performed by kea-lfc. It is the final step of the lease processing, however after this the kea-lfc process will continue by removing the working files and then move this file to Previous lease file.

4) In paragraph 2 of LFC process overview the documental has "Regardless if the Lease File copy exists or not," This should be "Regardless if the Lease File copy existed or not," as by this point we have renamed the current Lease File to the Lease File Copy.

5) Same paragraph has "it replaces moves the" which should be one or the other, "it replaces the".

6) In the section on kea-lfc. The code currently uses -p for the pid file and -x for the previous lease file.

7) In the section on preventing concurrent runs - having the server process restart and then kill the "LFC" process based on its PID is a bit dangerous. Because the server has been restarted it can't be sure that the current process with that PID is actually the LFC process and not some other process that has been started in the mean time. While we may go with choosing an specific option for now we may eventually want to add a configuration option to allow the administrator to decide what they want to happen - 1) kill the process, 2) wait for the process to finish (no matter how long), 3) wait for some duration and then delete the PID and start the LFC, 4) log an error and let the administrator sort it out.

I addressed your comments. Please check.

comment:8 Changed 5 years ago by sar

I did some editing to try and tidy up some typos.

I found a couple of more items that we could update to be completely consistent. I will mention them in the following but don't really think that fixing them is required.

In the LFC Process Overview section the last paragraph describes how a single file is processed. But it makes it sound like that file is processed and then the output file is written before the next file is processed instead of all files being read and then the output generated. This is described correctly elsewhere in the document.

In both the above section as well as the kea-lfc section the text describes replacing an entry for a client and at the end of reading having only a single entry per client. I'm not sure what Kea does currently but eventually we should be able to have multiple (active) leases per client. Consider the case of a client moving between different subnets - one client may have two valid active leases. I think the intention is to say we will have at most one entry per address rather than per client.

There is also a slight discrepancy with having one entry per client or address. In the current code if we have an address that is valid and then a second entry for that address but which has a 0 for the valid time field the address will be removed. As the address has expired this is fine but not exactly what the document claims.

comment:9 Changed 5 years ago by sar

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

As discussed with Marcin via Jabber. I think this design is not perfect but it is good enough.

Closing the ticket.

Note: See TracTickets for help on using tickets.