unioslo / tsdfx

File transfer utility
Other
4 stars 3 forks source link

scanner - log time #95

Closed angainor closed 8 years ago

angainor commented 8 years ago

add syslog that prints time it takes to perform a given scanner task

petterreinholdtsen commented 8 years ago

Look like the automatic testing no longer work with this patch. Did your 'make check' run work?

Looking at the code, I suspect the new log functions belong in log.h and tsd_log.c, not it a new file. It would be great if the code also had a test script to verify that it work. Can you add a test to t/ for this?

Finally, I believe it is more common in the current code to use start/stop idioms, and suggest to change the tic/toc function names to timer_start/timer_stop or something like that.

dag-erling commented 8 years ago

This is massively over-engineered. Just call gettimeofday() directly from tsdfx_scanner(). Also:

angainor commented 8 years ago

I've changed the timer function according to our discussion with Petter, and simplified them. I've also added a test case.

Dag-Erling: those and other profiling functions are going to be called in other places in the code (not only scanner).

Don't know why, but in the tests the copier fails if I create a directory in srcdir and make a file there. The directory itself is created in dstdir, but the file is not copied due to permission error. The case is included in my test-timing.sh, but commented out.

petterreinholdtsen commented 8 years ago

I suspect the subdir fail because tsdfx do not have any ordering of the requests, and will not delay trying to copy the file until the directory is created. the fill should be copied on the second iteration/scan.

angainor commented 8 years ago

I suspect the subdir fail because tsdfx do not have any ordering of the requests, and will not delay trying to copy the file until the directory is created. the fill should be copied on the second iteration/scan.

How can I check that? I've put a sleep 20 statement in the test file, nothing changed. d/test3 did not get copied, and directory permissions of dst/d are lacking 'x':

[marcink@angainor dst]$ ls -al total 16 drwxr-xr-x. 1 marcink users 3 Jun 20 15:37 . drwxr-xr-x. 1 marcink users 4 Jun 20 15:38 .. drw-------. 1 marcink users 0 Jun 20 15:37 d -rw-r-----. 1 marcink users 6 Jun 20 15:37 test1 -rw-r-----. 1 marcink users 6 Jun 20 15:37 test2

petterreinholdtsen commented 8 years ago

[angainor]

How can I check that? I've put a sleep 20 statement in the test file, nothing changed. d/test3 did not get copied, and directory permissions of dst/d are lacking 'x':

Hm, good catch. I expected it would be enough to run the master process several times, but this do not work. I tried this patch:

diff --git a/t/test-simplecopy.sh b/t/test-simplecopy.sh index 2c2bfa0..8c9d249 100755 --- a/t/test-simplecopy.sh +++ b/t/test-simplecopy.sh @@ -13,6 +13,8 @@ echo "badfile, should be ignored" > "${srcdir}/$(printf "\002")" echo test1 > "${srcdir}/test1" echo 'test(2)' > "${srcdir}/test (2).txt" echo test2 > "${srcdir}/test2" +mkdir "${srcdir}/d" +echo test3 > "${srcdir}/d/test3"

dd bs=1k count=${randomsize} \ if=/dev/urandom \ @@ -21,8 +23,10 @@ dd bs=1k count=${randomsize} \ md5start=$(cd ${srcdir}; md5sum ${randomsize}krandom)

run_daemon -1 +run_daemon -1 +run_daemon -1

-for good in test1 test2 ; do +for good in test1 test2 d/test3; do if [ ! -e "${dstdir}/${good}" ] ; then fail_test "missing: ${dstdir}/${good}" elif ! cmp -s "${srcdir}/${good}" "${dstdir}/${good}" ; then

I suspect this is a bug somewhere, and suggest we track this as a

separate issue. Creating it now.

Happy hacking Petter Reinholdtsen

petterreinholdtsen commented 8 years ago

A variation of this patch was commited as part of pull request #99. Thank you very much for the first draft. Closing this pull request. The logging is part of the new version released today, and will be put into production tonight.