Currently all the comination of createFlag and the expected behavior is not clearly defined in FileContext#create(). Here is my proposal on how the combination must behave the behavior a user needs:

CreateFlag specifies the file create semantic. Users can combine flags like:
EnumSet.of(CreateFlag.CREATE, CreateFlag.APPEND)

Use the CreateFlag as follows:
CREATE - to create a file if it does not exist, else throw FileAlreadyExists.
APPEND - to append to a file if it exists, else throw FileNotFoundException.
OVERWRITE - to truncate a file if it exists, else throw FileNotFoundException.
CREATE|APPEND - to create a file if it does not exist, else append to an existing file.
CREATE|OVERWRITE - to create a file if it does not exist, else overwrite an existing file.

Following combination is not valid and will result in HadoopIllegalArgumentException:
APPEND|OVERWRITE
CREATE|APPEND|OVERWRITE

Looks good.
BTW I wish we had used the Posix TRUNC instead of OVERWRITE since the rest of our flags are derived from Posix.
Please add a comment in the description that the semantics are same as TRUNC.

How about we add TRUNC, which is the same as OVERWRITE, and then deprecate OVERWRITE?

The problem with this is, all checks that compare objects for equality will fail in enums. I propose leaving it as it is, with documentation that describes that this is equivalent to O_TRUNC.

Changes:

1. Made changes equivalent to HADOOP-7223
2. Moved some of the functionality outside the lock.

> The problem with this is, all checks that compare objects for equality will fail in enums. ...

Ah, you are right. We could change the enum to a class but this is a bigger change that you probably won't like to do it here.

+1 patch looks good.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12476145/HDFS-1831.patch
against trunk revision 1091515.

+1 @author. The patch does not contain any @author tags.

-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 core tests. The patch failed these core unit tests:
org.apache.hadoop.cli.TestHDFSCLI
org.apache.hadoop.fs.loadGenerator.TestLoadGenerator
org.apache.hadoop.fs.permission.TestStickyBit
org.apache.hadoop.fs.TestHDFSFileContextMainOperations
org.apache.hadoop.fs.TestUrlStreamHandler
org.apache.hadoop.hdfs.server.balancer.TestBalancer
org.apache.hadoop.hdfs.server.datanode.TestBlockReplacement
org.apache.hadoop.hdfs.server.datanode.TestBlockReport
org.apache.hadoop.hdfs.server.datanode.TestDataNodeMetrics
org.apache.hadoop.hdfs.server.datanode.TestDatanodeRestart
org.apache.hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting
org.apache.hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure
org.apache.hadoop.hdfs.server.datanode.TestDataXceiver
org.apache.hadoop.hdfs.server.datanode.TestDirectoryScanner
org.apache.hadoop.hdfs.server.datanode.TestDiskError
org.apache.hadoop.hdfs.server.datanode.TestInterDatanodeProtocol
org.apache.hadoop.hdfs.server.datanode.TestTransferRbw
org.apache.hadoop.hdfs.server.datanode.TestWriteToReplica
org.apache.hadoop.hdfs.server.namenode.metrics.TestNameNodeMetrics
org.apache.hadoop.hdfs.server.namenode.metrics.TestNNMetricFilesInGetListingOps
org.apache.hadoop.hdfs.server.namenode.TestAllowFormat
org.apache.hadoop.hdfs.server.namenode.TestBackupNode
org.apache.hadoop.hdfs.server.namenode.TestBlocksWithNotEnoughRacks
org.apache.hadoop.hdfs.server.namenode.TestBlockTokenWithDFS
org.apache.hadoop.hdfs.server.namenode.TestBlockUnderConstruction
org.apache.hadoop.hdfs.server.namenode.TestCheckPointForSecurityTokens
org.apache.hadoop.hdfs.server.namenode.TestCheckpoint
org.apache.hadoop.hdfs.server.namenode.TestCorruptFilesJsp
org.apache.hadoop.hdfs.server.namenode.TestDeadDatanode
org.apache.hadoop.hdfs.server.namenode.TestDecommissioningStatus
org.apache.hadoop.hdfs.server.namenode.TestEditLogRace
org.apache.hadoop.hdfs.server.namenode.TestFileLimit
org.apache.hadoop.hdfs.server.namenode.TestFsck
org.apache.hadoop.hdfs.server.namenode.TestHDFSConcat
org.apache.hadoop.hdfs.server.namenode.TestLargeDirectoryDelete
org.apache.hadoop.hdfs.server.namenode.TestListCorruptFileBlocks
org.apache.hadoop.hdfs.server.namenode.TestMetaSave
org.apache.hadoop.hdfs.server.namenode.TestNameEditsConfigs
org.apache.hadoop.hdfs.server.namenode.TestNameNodeJspHelper
org.apache.hadoop.hdfs.server.namenode.TestNameNodeMXBean
org.apache.hadoop.hdfs.server.namenode.TestNNThroughputBenchmark
org.apache.hadoop.hdfs.server.namenode.TestNodeCount
org.apache.hadoop.hdfs.server.namenode.TestOverReplicatedBlocks
org.apache.hadoop.hdfs.server.namenode.TestParallelImageWrite
org.apache.hadoop.hdfs.server.namenode.TestSafeMode
org.apache.hadoop.hdfs.server.namenode.TestSecurityTokenEditLog
org.apache.hadoop.hdfs.server.namenode.TestStartup
org.apache.hadoop.hdfs.server.namenode.TestStorageRestore
org.apache.hadoop.hdfs.server.namenode.TestUnderReplicatedBlocks
org.apache.hadoop.hdfs.TestAbandonBlock
org.apache.hadoop.hdfs.TestBlockMissingException
org.apache.hadoop.hdfs.TestBlocksScheduledCounter
org.apache.hadoop.hdfs.TestClientBlockVerification
org.apache.hadoop.hdfs.TestClientProtocolForPipelineRecovery
org.apache.hadoop.hdfs.TestCrcCorruption
org.apache.hadoop.hdfs.TestDatanodeBlockScanner
org.apache.hadoop.hdfs.TestDatanodeDeath
org.apache.hadoop.hdfs.TestDataTransferProtocol
org.apache.hadoop.hdfs.TestDecommission
org.apache.hadoop.hdfs.TestDFSClientExcludedNodes
org.apache.hadoop.hdfs.TestDFSClientRetries
org.apache.hadoop.hdfs.TestDFSFinalize
org.apache.hadoop.hdfs.TestDFSMkdirs
org.apache.hadoop.hdfs.TestDfsOverAvroRpc
org.apache.hadoop.hdfs.TestDFSPermission
org.apache.hadoop.hdfs.TestDFSRemove
org.apache.hadoop.hdfs.TestDFSRename
org.apache.hadoop.hdfs.TestDFSRollback
org.apache.hadoop.hdfs.TestDFSShell
org.apache.hadoop.hdfs.TestDFSStartupVersions
org.apache.hadoop.hdfs.TestDFSStorageStateRecovery
org.apache.hadoop.hdfs.TestDFSUpgrade
org.apache.hadoop.hdfs.TestDistributedFileSystem
org.apache.hadoop.hdfs.TestFileAppend2
org.apache.hadoop.hdfs.TestFileAppend3
org.apache.hadoop.hdfs.TestFileAppend4
org.apache.hadoop.hdfs.TestFileAppend
org.apache.hadoop.hdfs.TestFileConcurrentReader
org.apache.hadoop.hdfs.TestFileCorruption
org.apache.hadoop.hdfs.TestFileCreationClient
org.apache.hadoop.hdfs.TestFileCreationDelete
org.apache.hadoop.hdfs.TestFileCreationEmpty
org.apache.hadoop.hdfs.TestFileCreationNamenodeRestart
org.apache.hadoop.hdfs.TestFileCreation
org.apache.hadoop.hdfs.TestFileStatus
org.apache.hadoop.hdfs.TestFSInputChecker
org.apache.hadoop.hdfs.TestFSOutputSummer
org.apache.hadoop.hdfs.TestGetBlocks
org.apache.hadoop.hdfs.TestHDFSFileSystemContract
org.apache.hadoop.hdfs.TestHDFSTrash
org.apache.hadoop.hdfs.TestHFlush
org.apache.hadoop.hdfs.TestHftpFileSystem
org.apache.hadoop.hdfs.TestInjectionForSimulatedStorage
org.apache.hadoop.hdfs.TestLargeBlock
org.apache.hadoop.hdfs.TestLeaseRecovery2
org.apache.hadoop.hdfs.TestLeaseRecovery
org.apache.hadoop.hdfs.TestLease
org.apache.hadoop.hdfs.TestListFilesInDFS
org.apache.hadoop.hdfs.TestListPathServlet
org.apache.hadoop.hdfs.TestLocalDFS
org.apache.hadoop.hdfs.TestMissingBlocksAlert
org.apache.hadoop.hdfs.TestModTime
org.apache.hadoop.hdfs.TestMultiThreadedHflush
org.apache.hadoop.hdfs.TestPipelines
org.apache.hadoop.hdfs.TestPread
org.apache.hadoop.hdfs.TestQuota
org.apache.hadoop.hdfs.TestReadWhileWriting
org.apache.hadoop.hdfs.TestRenameWhileOpen
org.apache.hadoop.hdfs.TestReplaceDatanodeOnFailure
org.apache.hadoop.hdfs.TestReplication
org.apache.hadoop.hdfs.TestRestartDFS
org.apache.hadoop.hdfs.TestSafeMode
org.apache.hadoop.hdfs.TestSeekBug
org.apache.hadoop.hdfs.TestSetrepDecreasing
org.apache.hadoop.hdfs.TestSetrepIncreasing
org.apache.hadoop.hdfs.TestSetTimes
org.apache.hadoop.hdfs.TestSmallBlock
org.apache.hadoop.hdfs.TestWriteConfigurationToDFS
org.apache.hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer
org.apache.hadoop.hdfs.tools.offlineImageViewer.TestOfflineImageViewer
org.apache.hadoop.security.TestPermission
org.apache.hadoop.security.TestRefreshUserMappings
org.apache.hadoop.tools.TestJMXGet

-1 contrib tests. The patch failed contrib unit tests.

+1 system test framework. The patch passed system test framework compile.

Test results: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/352//testReport/
Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/352//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/352//console

This message is automatically generated.

Updated patch retains the behavior of FileSystem where CreateFlag.OVERWRITE, creates a new file if file does not exist, else overwrites an existing file.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12476263/HDFS-1831.1.patch
against trunk revision 1091874.

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 6 new or modified tests.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 core tests. The patch failed these core unit tests:
org.apache.hadoop.fs.loadGenerator.TestLoadGenerator
org.apache.hadoop.fs.TestHDFSFileContextMainOperations
org.apache.hadoop.hdfs.server.datanode.TestBlockReport
org.apache.hadoop.hdfs.server.namenode.TestNNThroughputBenchmark
org.apache.hadoop.hdfs.TestFileConcurrentReader
org.apache.hadoop.hdfs.TestLeaseRecovery2

-1 contrib tests. The patch failed contrib unit tests.

+1 system test framework. The patch passed system test framework compile.

Test results: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/363//testReport/
Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/363//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/363//console

This message is automatically generated.

Additional changes in the attached patch:

1. Previously used OVERWRITE create flag is changed to CREATE|OVERWRITE, where required.
2. FileContext.create() with append options was failing. DFSClient#primitiveCreate() method is changed to use ClientProtocol#append() and to use the last block returned from it for append.
3. Cleaned up FSNamesystem#startFileInternal
   • Removed - Check for src being directory is done at the beginning of the method. This was unnecessarily done again.
   • Some additional cleanup.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12476387/HDFS-1831.4.patch
against trunk revision 1092534.

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 9 new or modified tests.

+1 javadoc. The javadoc tool did not generate any warning messages.

-1 javac. The patch appears to cause tar ant target to fail.

-1 findbugs. The patch appears to cause Findbugs (version 1.3.9) to fail.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 core tests. The patch failed these core unit tests:

-1 contrib tests. The patch failed contrib unit tests.

-1 system test framework. The patch failed system test framework compile.

Test results: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/371//testReport/
Console output: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/371//console

This message is automatically generated.

The Hudson build failed without the change it depends on in common available. Will trigger the hudson build once the common change is integrated.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12476387/HDFS-1831.4.patch
against trunk revision 1092534.

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 9 new or modified tests.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 core tests. The patch passed core unit tests.

-1 contrib tests. The patch failed contrib unit tests.

+1 system test framework. The patch passed system test framework compile.

Test results: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/373//testReport/
Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/373//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/373//console

This message is automatically generated.

Contrib test failure is due to TestAuthorizationFilter with an open jira HDFS-1666.

+1 patch looks good.

• Minor:
  We should also validate the flag in NameNode.

• An existing bug:
  NPE if appending to a non-existing file using FileSystem. FileContext does not have such problem.

> We should also validate the flag in NameNode.
I had to remove it to fix TestLeaseAppend2 failure.

TestLeaseAppend2 fails if the validation rejects the create request before going all the way to the end of FSNamesystem#startFileInternal(), through lease recovery code. This is a strange piece of code that we should address at some point.

Integrated in Hadoop-Hdfs-trunk-Commit #596 (See https://hudson.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/596/)
HDFS-1831. Fix append bug in FileContext and implement CreateFlag checks. Contributed by Suresh Srinivas.

I committed the patch.

Integrated in Hadoop-Hdfs-trunk #643 (See https://builds.apache.org/hudson/job/Hadoop-Hdfs-trunk/643/)