[#HADOOP-4494] libhdfs does not call FileSystem.append when O_APPEND passed to hdfsOpenFile

Hadoop Common

libhdfs does not call FileSystem.append when O_APPEND passed to hdfsOpenFile

Created: 22/Oct/08 08:38 PM Updated: 08/Jul/09 05:05 PM

Return to search

Component/s:

None

Affects Version/s:

0.19.1, 0.20.0

Fix Version/s:

0.19.1

Time Tracking:

Not Specified

File Attachments:

File Name	Date Attached	Attached By	Size
HADOOP-4494.txt	2008-10-23 09:29 PM	Pete Wyckoff	6 kB
HADOOP-4494.txt	2008-10-22 08:40 PM	Pete Wyckoff	6 kB

Issue Links:

Dependants

This issue depends on:

~~HADOOP-4508~~ FSDataOutputStream.getPos() == 0when appending to existing file and should be file length

Reference

This issue is related to:

HDFS-413 fuse-dfs fix writeTest and add writeAppendtest

Hadoop Flags:	Reviewed
Release Note:	libhdfs supports O_APPEND flag
Resolution Date:	11/Feb/09 11:04 AM

Description

« Hide

libhdfs currently calls FileSystem.create when O_APPEND passed in - ie it ignores this flag

All

Comments

Work Log

Change History

Subversion Commits

Sort Order:

[ Permlink | « Hide ]

dhruba borthakur added a comment - 22/Oct/08 09:12 PM

+1. Looks good.

[ Permlink | « Hide ]

Sanjay Radia added a comment - 22/Oct/08 10:42 PM

Minor doc suggestion:

In the documentation in hdfs.h
+ * @param flags Either O_RDONLY or O_WRONLY[|O_APPEND] for read-only or write-only.

O_WRONLY is for create not for writing to arbitrary part of a file.
Update the comment to indicate that O_WRONLY is for creating, etc.

I just noticed that the test uses O_CREAT but the impl does not even bother checking for this (ie treats O_WRONLY as create):
+++ src/c++/libhdfs/hdfs_test.c (working copy)
+ // CREATE
+ hdfsFile writeFile = hdfsOpenFile(fs, writePath, O_WRONLY|O_CREAT, 0, 0, 0);

Unfortunately fixing this to match posix will break compatibility of apps, since truck libhdfs did not check for O_CREAT either.

[ Permlink | « Hide ]

Pete Wyckoff added a comment - 23/Oct/08 12:34 AM

I just noticed that the test uses O_CREAT but the impl does not even bother checking for this (ie treats O_WRONLY as create):

Yes, open on writes should i guess be O_WRONLY + one of the following:

1. O_CREAT | O_EXCL - create new file
2. O_CREATE|O_APPEND - create new file if it doesn't exist, else append

But, as you point out hdfsOpenFile has only ever checked for O_WRONLY|O_RDONLY. And #2 cannot be done without underlying FileSystem API.

But, as you point out, if someone does something like `echo hello >> foo` where foo does not exist, libhdfs will do the wrong thing probably.

We need:

FSDataOutputStream FileSystem.appendIfExistsElseCreate(Path p);

I can open a JIRA for it. I guess that means anyone using libhdfs (e.g. fuse) would have to do a hacky check if the file exists and decide whether to pass on the append flag, which of course is not transactional.

And of course, let's not even get into passing on O_TRUNCATE But, the libhdfs code should probably check for that flag and error out if it comes.

[ Permlink | « Hide ]

Pete Wyckoff added a comment - 23/Oct/08 01:50 AM

idea - we deprecate not having the O_CREAT flag by writing out a warning and updating the docs.

The good thing about this kind of deprecation is we can ultimately just return -ENOENT when we are ready - should be easy for users to see they have to fix something.

but, should also look at O_EXCL at the same time ...