BIND 10 #1704: log output mixed

[BIND 10 Development]

#1704: log output mixed
-------------------------------------+-------------------------------------
                   Reporter:  jreed  |                 Owner:  jinmei
                       Type:         |                Status:  reviewing
  defect                             |             Milestone:
                   Priority:         |  Sprint-20120612
  medium                             |            Resolution:
                  Component:         |             Sensitive:  0
  logging                            |           Sub-Project:  Core
                   Keywords:         |  Estimated Difficulty:  15
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:63 muks]:

 In short, the branch now looks ready for merge.  Thanks for your
 patience:-)

 Responding to some selected points:

 > How does this look:
 > {{{
 > XYZ.    [func]*               muks
 >         The logger now uses a lockfile named `logger_lockfile' that is
 >         created in the local state directory to mutually separate
 >         individual logging operations from various processes. This is
 >         done so that log messages from different processes don't mix
 >         together in the middle of lines. The `logger_lockfile` is
 created
 >       with file permission mode 0660. BIND 10's local state directory
 >       should be writable and perhaps have g+s mode bit so that the
 >       `logger_lockfile` can be opened by a group of processes.
 > }}}

 Looks okay.

 > > '''bind10_src.py'''
 > >
 > > - Should we still use B10_FROM_BUILD for the lock path?
 > > {{{#!python
 > >     if "B10_FROM_BUILD" in os.environ:
 > >         lpath = os.environ["B10_FROM_BUILD"]
 > > }}}
 > >   See also the similar comment on interprocess_sync_file.cc.
 >
 > In both cases, I've retained B10_FROM_BUILD and override it with
 B10_LOCKFILE_DIR_FROM_BUILD, so that at least if B10_FROM_BUILD is set,
 it'll use the corresponding dir for creating lockfiles.

 Hmm, I'm not sure if it really makes sense, but I don't oppose.  In
 any case, we'll need to clarify the magic variables somewhere.

 > > - And what's the purpose of these unlinks?  Would something wrong
 > >   happen without these?  If so, it's generally better to do that by
 > >   introducing a fixture and doing the cleanup in the destructor or in
 > >   tearDown() and/or making sure the test starts cleanly by doing the
 > >   cleanup in the constructor or in setUp().
 >
 > The unlinks are used to cleanup (remove the test lockfiles). Without
 these, distcheck would complain about stale files that exist after make
 clean. They can be removed by a clean target, but in this case it's better
 if the test cleans up. Is a fixture absolutely necessary here?

 Hmm, it does not necessarily have to be a fixture, but, technically,
 adding cleanup code in the end of a test case is fragile because we
 might skip it due to ASSERT_xx failure (and the missing cleanup might
 cause unrelated false positives).  Also, it's more error prone because
 we could easily add another similar test case but forgetting the
 cleanup.  For the distcheck case you explained, the first reason
 doesn't matter because the check will fail anyway, but in general I
 prefer keeping it as safe as possible.

 That said, it's quite minor at this stage.  Let's keep it as it is and
 merge.  If we encounter a problem due to the way we clean it up we can
 then revisit it.

-- 
Ticket URL: <http://bind10.isc.org/ticket/1704#comment:65>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development