2012/1/18 Gregory Farnum <gregory.farnum@xxxxxxxxxxxxx>: > I pulled this patch into master. Thanks, Andrey! > > On Fri, Jan 13, 2012 at 4:04 PM, Noah Watkins <noahwatkins@xxxxxxxxx> wrote: >> On Fri, Jan 13, 2012 at 14:33, Andrey Stepachev <octo47@xxxxxxxxx> wrote: >>> 2012/1/14 Noah Watkins <noahwatkins@xxxxxxxxx>: >>>> On Fri, Jan 13, 2012 at 07:12, Andrey Stepachev <octo47@xxxxxxxxx> wrote: >>>>> From: Andrey Stepachev <octo@xxxxxxxxxxxxxx> >>>>> >>>>> Hadoop always see wrong dates due of wrong timestamp calculation. >>>>> Possibly fixes #1666. >>>> >>>> Using coarse-grained timestamps might mask the problem in 1666. But it >>>> would only work in certain circumstances such as when clock sync skew >>>> is sufficiently small, and I think edge conditions would continue to >>>> exist. >>> >>> No. It is really bug. Not a coarse grained solution. >>> Here is really bug, because if we don't divide we get >>> not milliseconds, we'll got seconds. In my test environment >>> i have time difference up to several hours. >>> >>> Java uses millis, so we must divide nanos to get millis. >>> java_mtime += st.st_mtim.tv_nsec / 1000; >>> >>>> A more robust solution is probably needed for #1666 :( >>> >>> How about new function ceph_lstat_nocache, which >>> in turn will call lstat with flags == 0. In that case >>> _get_attr always will query MDS (if I understand code, >>> but I can be wrong). It is a performance hit, but >>> hadoop should work. >>> We can ever do lstat twice and get the biggest >>> mtime. >> >> Yeh, we need some way to refresh the cached stats at the client (if i >> remember 1666 correctly). If you are running into this issue there is >> a small hack for Hadoop that you an apply (it's on the wiki) until >> something for 1666 is figured out. By the way, these JNI wrappers are >> being rewritten to be Hadoop agnostic (pure libcephfs), and the code >> is at github.com/noahdesu/java-cephfs -- I was just getting ready to >> port the stat code, so it is nice you brought this issue back up :) >> >>> >>>> >>>>> >>>>> Signed-off-by: Andrey Stepachev <octo@xxxxxxxxxxxxxx> >>>>> >>>>> --- >>>>> src/client/hadoop/CephFSInterface.cc | 8 ++++---- >>>>> 1 files changed, 4 insertions(+), 4 deletions(-) >>>>> >>>>> - long long java_mtime(st.st_mtime); >>>>> + long long java_mtime(st.st_mtim.tv_sec); >>>> >>>> mtime vs mtim: the later is non-portable? >>> >>> Yeah. Yes. This can be safely removed from patch. > > Is this something I should fix? It's presumably not a big problem > since the older code had references to "tv_mtim", but I'm not too > familiar with the history of *nix structs... I'm not a big guru too, but st_atim.tv_nsec used in code, so it looks more consistent to use st_atim.tv_sec, then st_atime (which is a macro). Accroding to stat.h if we havn't st_atim, code will not compile, because we try to access st_mtim.tv_nsec. So, I think that my original patch is preferable. > -Greg -- Andrey. -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
