wiki:WeeklyMinutes20120326

BIND 10 Team Call

2012-03-26

Attendees

Michal
Shane
Stephen
Kevin
Jelte
Mukund
Aharen
Jeremy
Kambe
Jinmei
Larissa

Agenda

We have 12 topics, which is 5 minutes per topic - NOT ENOUGH TIME.

End of Year Release Topics

This is our last call of the year, and right before our end-of-year release is branched. Any last minute issues should be discussed here.

Logging fix worked on by Stephen

New bug: #1836 -> add to current sprint, Jinmei will fix today

A few documentation things. In-memory explanation, will it be done? Jinmei picked it up, but if Jeremy wants to do that it is better.
Jeremy will work on that today.

Jeremy did not attend DHCP meeting in the past; calendar does not work; anything from DHCP side? Stephen: no Kea meeting tonight, will make sure on list of Kea meetings when back in UK. No DHCP issues for release.

Start of Year 4!

We're starting year 4!

BIND 10 Shutdown

This topic comes from ticket #1049, which issues SIGKILL after processes take too long to shut down. How exactly should the BIND 10 shutdown work? Some sub-topics:

  • Normal shutdown
  • Abnormal shutdown
  • Waiting for programs to end
  • SIGTERM?

Jinmei: My understanding of the problem is that it might cause broken database or something. It's actually not an issue of whether to use SIGKILL or anything, but more about how we insure graceful shutdown; by which I mean without leaving any inconsistent state. As long as we can reach that goal it doesn't matter.
Michal: I think we cannot leave database in inconsistent state no matter how we shut down because someone may unplug the server or something.
Mukund: Do we have to kill processes? Can we not wait for them to shutdown? Normally a well-behaving process will shut down.
Michal: The point is if it is in a loop or something then the administrator will be angry.
Mukund: At that point it is not a graceful shutdown or not. You don't expect things to be in an inconsistent state, so we cannot assume that it will shutdown properly.
Shane: We could create a way to say "I am shutting down", but that seems complicated to me.
Michal don't think we should be shutting down for more than a second.

Mukund: Maybe we need a way to administratively really shut down.
Jinmei: BIND 9 has a concept like that.

Jinmei: Another random idea. We could ask the user via bindctl "the shutdown process takes time do you want to kill it immediately or wait?"
Michal: That is complicated, we may have terminated cmdctl already.
Jinmei: Just thinking of the feature, not implementation.

Jeremy: We need to consider the clustering of this. Multiple users may be using bindctl. To have it shutdown without one user's task completing.
Jeremy: I consider we make the shutdown 5 or 10 seconds by default.
Jelte: I was thinking along the same lines, and if it doesn't end in a second or two that is a bug and we should be running in "--no-kill" (we developers).

Jinmei: It is frustrating to wait too long after saying "shutdown". We have not had broken state due to kill so far. So I only see unproven possibility about the concern.
Shane: Another possibility is just to do SIGKILL all the time.
Jeremy: We have lost log messages. SIGKILL stopped that.
Jinmei: If we can think about the things that might be lost, we can think about the things that might be lost. In this case it is a nice compromise if it makes the shutdown speed fast. Only worry about something unseen doesn't seem to be constructive.
Jeremy: For example, a default shutdown with BIND 9 will allow a task to complete, for example saving a zone transfer. Apache also waits for tasks to complete with a graceful shutdown.

AP: Shane to type this discussion up and send to list.

Mukund: "--no-kill" / "-i" is set, if you have preference about this then put it in the chat room and we'll use the most common.

Bikeshed: Operator Overloading in C++

https://lists.isc.org/pipermail/bind10-dev/2012-February/003113.html I have specific proposals: 

Jinmei: Two points:

  1. deprecate almost-unused rule (overloaded and non-overloading method also)
  2. we should be careful about overloading, and review process should require a discussion of operator overloading

Shane: Don't remember why we had the overloaded+non-overloaded guideline (even if I proposed it). Don't mind deprecating if we don't use it.
[all agree]

AP: Jinmei will update the coding guidelines.

Usage of @ in python doxygen

From #963. Because \ means something inside """documentation comment"""

Michal: When we use doxygen in Python we have two choices, \ or @ for keywords. If it is written in a docstring then \ means something. So we should be using the @-sign there. Any concerns?

Jinmei: I have no concerns about this one, but I don't know whether we already decided to use doxygen for Python.
Shane: We did.
Michal: Stephen documented some Python code with doxygen, because it is more consistent and forces him to do the documentation better.
Jinmei: The generated HTML did not look good to me. I didn't know we decided. If all of the others are fine with that then I do not necessarily object to that, but I am not sure if that is a good choice.
Jelte: I have no strong opinion.
Michal: I don't use the documentation from documentation too much - I usually just read the function.
Stephen: Then why are we using doxygen at all then?
Jinmei: For c++?
Stephen: If you're looking at the file, maybe it would be better to have more free-form commenting.
Michal: I don't think it makes sense to change it with so much code documented.
Stephen: I agree, but if we have C++ with doxygen, why aren't we documenting Python with doxygen.
Jinmei: I do not push pydoc, but you can also check with the Python interpreter. With doxygen you need to transform it to some other form. If it looks like I have no objection to that, but it did not look sophisticated to me.
Michal: If you put the doxygen into the """ """ you can see it inline, but you will still have the @ and stuff like that.
Jelte: Can you check parameters?
Shane: I believe so.
Michal: I don't think so.
Shane: I think so, but not sure. If you want stuff that really looks good then the Sphinx stuff is nice.

Jinmei: If we're going to use doxygen I have no problem with @.
Shane: Maybe a topic for face-to-face, although also maybe need homework before then.

Bikeshed: "variable == value" vs "value == variable"

https://lists.isc.org/pipermail/bind10-dev/2012-March/003266.html

AP: Jinmei will update guidelines.

Quality of commit log messages

Commit log messages such as "fixes bug XYZ" and "fixes for review comments" are not useful. Bad commit log messages are a problem when you're trying to debug something by looking at old commits and finding out why something was changed the way it was. (I'm typing this here, because this happened today.)

The Linux kernel commit logs are a good example of how commit messages should be written. The commits in a repository should be readable independently of any other system such as bug trackers, wiki pages, etc. Where applicable, the commit log must explain the rationale for the change (the 'why' part) and what it is fixing independent of any bugs system. Someone will be reading these log messages many years after you've written them to figure out where a problem was introduced or removed.

We can't make rules here. We should probably make a page of good and bad commit message examples, and reviewers should review commit messages too.

(We discussed this before but now I don't see the policy documented in the wiki.)

Michal: Should we fix this by re-writing the history or something?
Mukund: That would mean re-writing the branch. I don't know.
Jinmei: I think it's a good idea to improve the quality of commit log messages in general. I simply am not sure how exactly we do this. Whether it is up to the developer, or whether we have some level of guidelines for doing this. And whether we want to include this in the review process.

Shane: I think the review process is the natural place for this. I also am not sure we can have many guidelines for this.
Jeremy: Also we saw sometimes saying "checkpoint".
Michal: I think we can do this rewriting in a branch, updating the commits.
Jelte: We can branch the branch before we merge, and then merge the branched branch after we re-base it.
Jinmei: I'm not sure how we do this effectively.

Shane: If we assume normally the messages are fine and we only need to change the log messages rarely then it should not be that much overhead.
Michal: git-rebase --interactive will ask you what to do for each of the commits. If you use this to reword each you can edit each commit message.
Shane: That sounds alright.
Jelte: If you've used the script to automatically prepend the ticket number it does some strange things.

Jinmei: Can we have some specific suggestions, on the mailing list?
Jelte: Some examples?
Jinmei: Yes. What specifically we do. I'm not really sure beyond the idea.

AP: Mukund will get some examples together and send to the list. 

Mukund: I think once we've done this for some time we won't need to do this rebasing too much. It won't be a big overhead eventually.
Jeremy: As an easy step we can have a simple policy that says "no single-line log message that says 'addressed review comments'".

Estimating New Tickets

Currently we do "just in time" estimation - Jelte puts together some tickets to estimate before each sprint. However, it might make more sense to estimate all tasks. Lets discuss whether we want to do this, and how.

Michal: I don't know if we want to do that or not. But the technical solution is a problem if the ticket is a problem if the ticket is created in a different milestone than "new tasks".
Shane: That's a problem anyway, because it kind of gets lost.

Jinmei: In principle it is a good idea. My question is whether we can do it effectively.
Mukund: Isn't the point system relative. How we are feeling that particular week.
Shane: It's meant to be more-or-less stable over time, but perhaps really old estimates need to be consideration.
Jeremy: Currently tickets are explained good enough to make estimation. But if you have all tickets estimated then they are not explained.
Shane: The idea is that I would continue to do initial triage like today, and then pass them on for estimation.

Shane: At least initially it would be more estimation, which may be too much.
Jinmei: That's my concern too. If we can control that then I would have no problem.
Shane: Maybe I can try to cap the maximum number, and try to get more pre-estimation.

AP: Shane will talk with Jelte and try to figure out an effective way to do this.

Logging output at startup

What to do about logging at startup? Even if all logging is configured to go to a defined file destination, at startup (-verbose) 130 lines were still output to console. This is noisy and the admin will need own way to manage these separate logs. This is why I never provided my rc.d startup script since we don't know what logging is done. One idea is to make default be a file or syslog, but even then if that is different than what is configured then admin will maintain two different sets of logs. I think it makes troubleshooting confusing also. One solution may be to only report fatal error to console and everything is buffered until logging is up. A similar topic was on agenda for http://bind10.isc.org/wiki/WeeklyMinutes20111214 but I don't think was ever fully discussed then.

Michal: I had an idea some time ago. It is to use a ring buffer - we log to memory and then dump it later on, which can be used to dump to console or whatever is configured.
Shane: The problem is that an error may result in no output.
Michal: We can log to console and ring buffer, then log to file so file is complete.
Jinmei: Current logging is too noisy. Using something like temporary memory buffer is one promising idea for this.

Shane: Should we create a ticket for that?
Michal: Maybe a discussion on the mailing list? Or a ticket to do the design.
Jeremy: We can do this on the list. A new ticket won't get handled for another week.

AP: Michal will send proposal with details in mind.

Out of time. Over at 16:00 UTC.


Disabled tests

Should we have failures for some percentage or amount of disabled tests? (I got this idea from the book "Continuous Delivery".) Or should we just have a periodic (monthly) reminder about disabled tests?

Doxygen warnings

Okay to turn on failure reports for doxygen warnings now? (Also who will handle #1531 which has some cleanup?)  We keep putting this off due to not ready, but we also don't have incentive to do it.

How to review tasks done by new developers

There are several things new developers often miss (common examples are detailed tests or documentation).  Normally we expect these can be caught and addressed in review, but if the reviewer is also a new developer, the reviewer may miss these things, too.  It would be better to make the review process fail-safe while still encouraging contributions from new developers.  Imposing a rule like "either the coder or reviewer must be an experienced project member" sounds too strict to me, especially because we have time difference issues.  Another possibility might be if both are new developers at least one experienced member performs higher level checks after review (not looking into the code, but check overall quality/coverage of tests and documentation, etc).

Last modified 6 years ago Last modified on Mar 26, 2012, 6:19:18 PM