Skip to content

Commit

Permalink
Remove the usage of must_cache and async_cache
Browse files Browse the repository at this point in the history
### What changes are proposed in this pull request?
current version didn't support must_cache and async_cache, so remove the usage of must_cache and async_cache usage in code and mark deprecated to prevent any use.

### Why are the changes needed?
This change focuses on removing the write type of MUST_CACHE and ASYNC_CACHE, and #17980 would remove the master-side code about async persist.

### Does this PR introduce any user facing changes?
No

			pr-link: #17963
			change-id: cid-14d40a217134beb8a5e7316b050af01bca18b65e
  • Loading branch information
Jackson-Wang-7 authored Aug 16, 2023
1 parent a3495ee commit 6dc8ed2
Show file tree
Hide file tree
Showing 64 changed files with 172 additions and 620 deletions.
2 changes: 1 addition & 1 deletion common/transport/src/main/proto/grpc/block_worker.proto
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ message CopyResponse {

message WriteOptions {
optional bool overwrite = 1;
optional grpc.file.WritePType write_type = 2;
optional grpc.file.WritePType write_type = 2 [default = CACHE_THROUGH];
optional bool check_content = 3;
}

Expand Down
14 changes: 7 additions & 7 deletions common/transport/src/main/proto/grpc/file_system_master.proto
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ import "grpc/fscommon.proto";
import "proto/journal/file.proto";

enum WritePType {
MUST_CACHE = 1;
TRY_CACHE = 2;
MUST_CACHE = 1 [deprecated = true];
TRY_CACHE = 2 [deprecated = true];
CACHE_THROUGH = 3;
THROUGH = 4;
ASYNC_THROUGH = 5;
ASYNC_THROUGH = 5 [deprecated = true];
NONE = 6;
}

Expand Down Expand Up @@ -117,7 +117,7 @@ message CreateDirectoryPOptions {
optional bool recursive = 1;
optional bool allowExists = 2;
optional PMode mode = 3;
optional WritePType writeType = 4;
optional WritePType writeType = 4 [default = CACHE_THROUGH];
optional FileSystemMasterCommonPOptions commonOptions = 5;
map<string, bytes> xattr = 6;
optional XAttrPropagationStrategy xattrPropStrat = 7 [default = NEW_PATHS];
Expand All @@ -142,7 +142,7 @@ message CreateFilePOptions {
optional int32 replicationMin = 5;
optional int32 replicationDurable = 6;
optional int32 writeTier = 7;
optional WritePType writeType = 8;
optional WritePType writeType = 8 [default = CACHE_THROUGH];
optional FileSystemMasterCommonPOptions commonOptions = 9;
optional int64 persistenceWaitTime = 10;
map<string, bytes> xattr = 11;
Expand Down Expand Up @@ -614,7 +614,7 @@ message CopyJobPOptions {
optional bool verify = 2;
optional bool partialListing = 3;
optional bool overwrite = 4;
optional WritePType writeType = 5;
optional WritePType writeType = 5 [default = CACHE_THROUGH];
optional bool check_content = 6;
}

Expand All @@ -623,7 +623,7 @@ message MoveJobPOptions {
optional bool verify = 2;
optional bool partialListing = 3;
optional bool overwrite = 4;
optional WritePType writeType = 5;
optional WritePType writeType = 5 [default = CACHE_THROUGH];
optional bool check_content = 6;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

package alluxio.client.block.stream;

import alluxio.client.WriteType;
import alluxio.client.file.FileSystemContext;
import alluxio.client.file.options.OutStreamOptions;
import alluxio.conf.AlluxioConfiguration;
Expand Down Expand Up @@ -44,7 +43,6 @@ public final class BlockWorkerDataWriter implements DataWriter {
private final BlockWriter mBlockWriter;
private final BlockWorker mBlockWorker;
private final int mChunkSize;
private final OutStreamOptions mOptions;
private final long mSessionId;
private final long mBufferSize;
private final long mReservedBytes;
Expand Down Expand Up @@ -132,8 +130,7 @@ public void flush() {}
public void close() throws IOException {
mBlockWriter.close();
try {
mBlockWorker.commitBlock(mSessionId, mBlockId,
mOptions.getWriteType() == WriteType.ASYNC_THROUGH);
mBlockWorker.commitBlock(mSessionId, mBlockId, false);
} catch (Exception e) {
mBlockWorker.cleanupSession(mSessionId);
throw new IOException(e);
Expand All @@ -157,7 +154,6 @@ private BlockWorkerDataWriter(long sessionId, long blockId, OutStreamOptions opt
mBlockWriter = blockWriter;
mChunkSize = chunkSize;
mBlockId = blockId;
mOptions = options;
mSessionId = sessionId;
mReservedBytes = reservedBytes;
mBufferSize = conf.getBytes(PropertyKey.USER_FILE_BUFFER_BYTES);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

package alluxio.client.block.stream;

import alluxio.client.WriteType;
import alluxio.client.file.FileSystemContext;
import alluxio.client.file.options.OutStreamOptions;
import alluxio.conf.AlluxioConfiguration;
Expand Down Expand Up @@ -135,7 +134,6 @@ private GrpcDataWriter(FileSystemContext context, final WorkerNetAddress address
builder.setCreateUfsFileOptions(ufsFileOptions);
}
// check if we need to pin block on create
builder.setPinOnCreate(options.getWriteType() == WriteType.ASYNC_THROUGH);
builder.setSpaceToReserve(reservedBytes);
mPartialRequest = builder.buildPartial();
mChunkSize = chunkSize;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
*/
@ThreadSafe
public final class FileSystemTestUtils {
public static final WritePType DEFAULT_WRITE_TYPE = WritePType.CACHE_THROUGH;

/**
* Creates a simple file with {@code len} bytes.
*
Expand All @@ -45,6 +47,17 @@ public static void createByteFile(FileSystem fs, String fileName, int len,
createByteFile(fs, new AlluxioURI(fileName), options, len);
}

/**
* Creates a simple file with {@code len} bytes.
*
* @param fs a {@link FileSystem} handler
* @param fileName the name of the file to be created
* @param len file size
*/
public static void createByteFile(FileSystem fs, String fileName, int len) {
createByteFile(fs, new AlluxioURI(fileName), DEFAULT_WRITE_TYPE, len);
}

/**
* Creates a simple file with {@code len} bytes.
*
Expand All @@ -58,6 +71,17 @@ public static void createByteFile(FileSystem fs, String fileName,
createByteFile(fs, new AlluxioURI(fileName), writeType, len);
}

/**
* Creates a simple file with {@code len} bytes.
*
* @param fs a {@link FileSystem} handler
* @param fileURI URI of the file
* @param len file size
*/
public static void createByteFile(FileSystem fs, AlluxioURI fileURI, int len) {
createByteFile(fs, fileURI, DEFAULT_WRITE_TYPE, len);
}

/**
* Creates a simple file with {@code len} bytes.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ public enum UnderStorageType {

/** Do not persist data to the under storage. */
NO_PERSIST(2),

/** Persist data to the under storage asynchronously. */
ASYNC_PERSIST(3),
;

private final int mValue;
Expand All @@ -42,11 +39,4 @@ public enum UnderStorageType {
public boolean isSyncPersist() {
return mValue == SYNC_PERSIST.mValue;
}

/**
* @return whether the data should be persisted to the under storage asynchronously
*/
public boolean isAsyncPersist() {
return mValue == ASYNC_PERSIST.mValue;
}
}
24 changes: 9 additions & 15 deletions dora/core/common/src/main/java/alluxio/client/WriteType.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,16 @@ public enum WriteType {
* Write the file, guaranteeing the data is written to Alluxio storage or failing the operation.
* The data will be written to the highest tier in a worker's storage. Data will not be
* persisted to the under storage.
*
* @deprecated This write type is deprecated as of v3.0 and not recommended for use.
*/
@Deprecated
MUST_CACHE(1),
/**
* Write the file and try to cache it.
*
* @deprecated This write type is deprecated as of v0.8 and not recommended for use. Use either
* {@link #MUST_CACHE} or {@link #CACHE_THROUGH} depending on your data persistence
* {@link #CACHE_THROUGH} or {@link #THROUGH} depending on your data persistence
* requirements.
*/
@Deprecated
Expand All @@ -48,7 +51,10 @@ public enum WriteType {
THROUGH(4),
/**
* [Experimental] Write the file asynchronously to the under fs.
*
* @deprecated This write type is deprecated as of v3.0 and not recommended for use.
*/
@Deprecated
ASYNC_THROUGH(5),
/**
* Do not store the data in Alluxio or Under Storage. This write type should only be used for
Expand Down Expand Up @@ -79,8 +85,6 @@ public AlluxioStorageType getAlluxioStorageType() {
public UnderStorageType getUnderStorageType() {
if (isThrough()) {
return UnderStorageType.SYNC_PERSIST;
} else if (isAsync()) {
return UnderStorageType.ASYNC_PERSIST;
}
return UnderStorageType.NO_PERSIST;
}
Expand All @@ -92,22 +96,12 @@ public int getValue() {
return mValue;
}

/**
* @return true if by this write type data will be persisted <em>asynchronously</em> to under
* storage (e.g., {@link #ASYNC_THROUGH}), false otherwise
*/
public boolean isAsync() {
return mValue == ASYNC_THROUGH.mValue;
}

/**
* @return true if by this write type data will be cached in Alluxio space (e.g.,
* {@link #MUST_CACHE}, {@link #CACHE_THROUGH}, {@link #TRY_CACHE}, or
* {@link #ASYNC_THROUGH}), false otherwise
* {@link #CACHE_THROUGH}, {@link #TRY_CACHE}, false otherwise
*/
public boolean isCache() {
return (mValue == MUST_CACHE.mValue) || (mValue == CACHE_THROUGH.mValue)
|| (mValue == TRY_CACHE.mValue) || (mValue == ASYNC_THROUGH.mValue);
return (mValue == CACHE_THROUGH.mValue) || (mValue == TRY_CACHE.mValue);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
package alluxio.master.file.meta;

import alluxio.AlluxioURI;
import alluxio.client.WriteType;
import alluxio.collections.Pair;
import alluxio.concurrent.LockMode;
import alluxio.conf.Configuration;
Expand Down Expand Up @@ -1034,9 +1033,6 @@ public List<Inode> createPath(RpcContext rpcContext, LockedInodePath inodePath,
if (fileContext.isCacheable()) {
newFile.setCacheable(true);
}
if (fileContext.getWriteType() == WriteType.ASYNC_THROUGH) {
newFile.setPersistenceState(PersistenceState.TO_BE_PERSISTED);
}
if (context.getXAttr() != null) {
newFile.setXAttr(context.getXAttr());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import alluxio.AuthenticatedClientUserResource;
import alluxio.AuthenticatedUserRule;
import alluxio.Constants;
import alluxio.client.WriteType;
import alluxio.annotation.dora.DoraTestTodoItem;
import alluxio.conf.Configuration;
import alluxio.exception.AccessControlException;
import alluxio.exception.BlockInfoException;
Expand Down Expand Up @@ -63,12 +63,12 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Sets;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

import java.io.Closeable;
import java.io.File;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.ArrayList;
Expand All @@ -78,25 +78,16 @@
import java.util.Set;
import java.util.stream.Collectors;

@DoraTestTodoItem(action = DoraTestTodoItem.Action.REMOVE, owner = "jiacheng",
comment = "remove FileSystemMaster unit testcases")
@Ignore
@RunWith(Parameterized.class)
public class FileSystemMasterFsOptsTest extends FileSystemMasterTestBase {

public FileSystemMasterFsOptsTest(InodeStore.Factory factory) {
mInodeStoreFactory = factory;
}

@Test
public void createFileMustCacheThenCacheThrough() throws Exception {
File file = mTestFolder.newFile();
AlluxioURI path = new AlluxioURI("/test");
mFileSystemMaster.createFile(path,
CreateFileContext.defaults().setWriteType(WriteType.MUST_CACHE));

mThrown.expect(FileAlreadyExistsException.class);
mFileSystemMaster.createFile(path,
CreateFileContext.defaults().setWriteType(WriteType.MUST_CACHE));
}

@Test
public void createFileUsesOperationTime() throws Exception {
AlluxioURI path = new AlluxioURI("/test");
Expand Down
Loading

0 comments on commit 6dc8ed2

Please sign in to comment.