Skip to content

Commit

Permalink
Replacing client-side SetAttributeOptions with server-side version an…
Browse files Browse the repository at this point in the history
…d renaming SetStateEntry to SetAttributeEntry.
  • Loading branch information
jsimsa committed Feb 3, 2016
1 parent b569562 commit 9f2f337
Show file tree
Hide file tree
Showing 15 changed files with 471 additions and 247 deletions.
10 changes: 5 additions & 5 deletions core/server/src/main/java/tachyon/master/MasterSource.java
Expand Up @@ -89,8 +89,8 @@ public class MasterSource implements Source {
mMetricRegistry.counter(MetricRegistry.name("MountOps")); mMetricRegistry.counter(MetricRegistry.name("MountOps"));
private final Counter mRenamePathOps = private final Counter mRenamePathOps =
mMetricRegistry.counter(MetricRegistry.name("RenamePathOps")); mMetricRegistry.counter(MetricRegistry.name("RenamePathOps"));
private final Counter mSetStateOps = private final Counter mSetAttributeOps =
mMetricRegistry.counter(MetricRegistry.name("SetStateOps")); mMetricRegistry.counter(MetricRegistry.name("SetAttributeOps"));
private final Counter mUnmountOps = private final Counter mUnmountOps =
mMetricRegistry.counter(MetricRegistry.name("UnmountOps")); mMetricRegistry.counter(MetricRegistry.name("UnmountOps"));


Expand Down Expand Up @@ -403,12 +403,12 @@ public void incRenamePathOps(long n) {
} }


/** /**
* Increments the counter of set state RPCs. * Increments the counter of set attribute RPCs.
* *
* @param n the increment * @param n the increment
*/ */
public void incSetStateOps(long n) { public void incSetAttributeOps(long n) {
mSetStateOps.inc(n); mSetAttributeOps.inc(n);
} }


/** /**
Expand Down
53 changes: 26 additions & 27 deletions core/server/src/main/java/tachyon/master/file/FileSystemMaster.java
Expand Up @@ -42,7 +42,6 @@


import tachyon.Constants; import tachyon.Constants;
import tachyon.TachyonURI; import tachyon.TachyonURI;
import tachyon.client.file.options.SetAttributeOptions;
import tachyon.collections.Pair; import tachyon.collections.Pair;
import tachyon.collections.PrefixList; import tachyon.collections.PrefixList;
import tachyon.conf.TachyonConf; import tachyon.conf.TachyonConf;
Expand Down Expand Up @@ -78,6 +77,7 @@
import tachyon.master.file.options.CreateDirectoryOptions; import tachyon.master.file.options.CreateDirectoryOptions;
import tachyon.master.file.options.CreateFileOptions; import tachyon.master.file.options.CreateFileOptions;
import tachyon.master.file.options.SetAclOptions; import tachyon.master.file.options.SetAclOptions;
import tachyon.master.file.options.SetAttributeOptions;
import tachyon.master.journal.Journal; import tachyon.master.journal.Journal;
import tachyon.master.journal.JournalOutputStream; import tachyon.master.journal.JournalOutputStream;
import tachyon.master.journal.JournalProtoUtils; import tachyon.master.journal.JournalProtoUtils;
Expand All @@ -94,7 +94,7 @@
import tachyon.proto.journal.File.ReinitializeFileEntry; import tachyon.proto.journal.File.ReinitializeFileEntry;
import tachyon.proto.journal.File.RenameEntry; import tachyon.proto.journal.File.RenameEntry;
import tachyon.proto.journal.File.SetAclEntry; import tachyon.proto.journal.File.SetAclEntry;
import tachyon.proto.journal.File.SetStateEntry; import tachyon.proto.journal.File.SetAttributeEntry;
import tachyon.proto.journal.Journal.JournalEntry; import tachyon.proto.journal.Journal.JournalEntry;
import tachyon.security.User; import tachyon.security.User;
import tachyon.security.authentication.PlainSaslServer; import tachyon.security.authentication.PlainSaslServer;
Expand Down Expand Up @@ -248,9 +248,9 @@ public void processJournalEntry(JournalEntry entry) throws IOException {
} catch (FileAlreadyCompletedException e) { } catch (FileAlreadyCompletedException e) {
throw new RuntimeException(e); throw new RuntimeException(e);
} }
} else if (innerEntry instanceof SetStateEntry) { } else if (innerEntry instanceof SetAttributeEntry) {
try { try {
setStateFromEntry((SetStateEntry) innerEntry); setAttributeFromEntry((SetAttributeEntry) innerEntry);
} catch (FileDoesNotExistException e) { } catch (FileDoesNotExistException e) {
throw new RuntimeException(e); throw new RuntimeException(e);
} }
Expand Down Expand Up @@ -1705,34 +1705,34 @@ public void resetFile(long fileId)
} }


/** /**
* Sets the file state. * Sets the file attribute.
* *
* @param path the path to set state * @param path the path to set attribute for
* @param options attributes to be set, see {@link SetAttributeOptions} * @param options attributes to be set, see {@link SetAttributeOptions}
* @throws FileDoesNotExistException if the file does not exist * @throws FileDoesNotExistException if the file does not exist
* @throws AccessControlException if permission checking fails * @throws AccessControlException if permission checking fails
* @throws InvalidPathException if the given path is invalid * @throws InvalidPathException if the given path is invalid
*/ */
public void setState(TachyonURI path, SetAttributeOptions options) public void setAttribute(TachyonURI path, SetAttributeOptions options)
throws FileDoesNotExistException, AccessControlException, InvalidPathException { throws FileDoesNotExistException, AccessControlException, InvalidPathException {
MasterContext.getMasterSource().incSetStateOps(1); MasterContext.getMasterSource().incSetAttributeOps(1);
synchronized (mInodeTree) { synchronized (mInodeTree) {
checkPermission(FileSystemAction.WRITE, path, false); checkPermission(FileSystemAction.WRITE, path, false);
long fileId = mInodeTree.getInodeByPath(path).getId(); long fileId = mInodeTree.getInodeByPath(path).getId();
long opTimeMs = System.currentTimeMillis(); long opTimeMs = System.currentTimeMillis();
setStateInternal(fileId, opTimeMs, options); setAttributeInternal(fileId, opTimeMs, options);
SetStateEntry.Builder setState = SetAttributeEntry.Builder builder =
SetStateEntry.newBuilder().setId(fileId).setOpTimeMs(opTimeMs); SetAttributeEntry.newBuilder().setId(fileId).setOpTimeMs(opTimeMs);
if (options.hasPinned()) { if (options.getPinned() != null) {
setState.setPinned(options.getPinned()); builder.setPinned(options.getPinned());
} }
if (options.hasTtl()) { if (options.getTtl() != null) {
setState.setTtl(options.getTtl()); builder.setTtl(options.getTtl());
} }
if (options.hasPersisted()) { if (options.getPersisted() != null) {
setState.setPersisted(options.getPersisted()); builder.setPersisted(options.getPersisted());
} }
writeJournalEntry(JournalEntry.newBuilder().setSetState(setState).build()); writeJournalEntry(JournalEntry.newBuilder().setSetAttribute(builder).build());
flushJournal(); flushJournal();
} }
} }
Expand Down Expand Up @@ -1891,7 +1891,7 @@ private List<PersistFile> pollFilesToCheckpoint(long workerId)
public synchronized FileSystemCommand workerHeartbeat(long workerId, List<Long> persistedFiles) public synchronized FileSystemCommand workerHeartbeat(long workerId, List<Long> persistedFiles)
throws FileDoesNotExistException, InvalidPathException, AccessControlException { throws FileDoesNotExistException, InvalidPathException, AccessControlException {
for (long fileId : persistedFiles) { for (long fileId : persistedFiles) {
setState(getPath(fileId), SetAttributeOptions.defaults().setPersisted(true)); setAttribute(getPath(fileId), new SetAttributeOptions.Builder().setPersisted(true).build());
} }


// get the files for the given worker to checkpoint // get the files for the given worker to checkpoint
Expand All @@ -1912,14 +1912,14 @@ public synchronized FileSystemCommand workerHeartbeat(long workerId, List<Long>
* @param options the method options * @param options the method options
* @throws FileDoesNotExistException * @throws FileDoesNotExistException
*/ */
private void setStateInternal(long fileId, long opTimeMs, SetAttributeOptions options) private void setAttributeInternal(long fileId, long opTimeMs, SetAttributeOptions options)
throws FileDoesNotExistException { throws FileDoesNotExistException {
Inode inode = mInodeTree.getInodeById(fileId); Inode inode = mInodeTree.getInodeById(fileId);
if (options.hasPinned()) { if (options.getPinned() != null) {
mInodeTree.setPinned(inode, options.getPinned(), opTimeMs); mInodeTree.setPinned(inode, options.getPinned(), opTimeMs);
inode.setLastModificationTimeMs(opTimeMs); inode.setLastModificationTimeMs(opTimeMs);
} }
if (options.hasTtl()) { if (options.getTtl() != null) {
Preconditions.checkArgument(inode.isFile(), PreconditionMessage.TTL_ONLY_FOR_FILE); Preconditions.checkArgument(inode.isFile(), PreconditionMessage.TTL_ONLY_FOR_FILE);
long ttl = options.getTtl(); long ttl = options.getTtl();
InodeFile file = (InodeFile) inode; InodeFile file = (InodeFile) inode;
Expand All @@ -1930,7 +1930,7 @@ private void setStateInternal(long fileId, long opTimeMs, SetAttributeOptions op
file.setLastModificationTimeMs(opTimeMs); file.setLastModificationTimeMs(opTimeMs);
} }
} }
if (options.hasPersisted()) { if (options.getPersisted() != null) {
Preconditions.checkArgument(inode.isFile(), PreconditionMessage.PERSIST_ONLY_FOR_FILE); Preconditions.checkArgument(inode.isFile(), PreconditionMessage.PERSIST_ONLY_FOR_FILE);
Preconditions.checkArgument(((InodeFile) inode).isCompleted(), Preconditions.checkArgument(((InodeFile) inode).isCompleted(),
PreconditionMessage.FILE_TO_PERSIST_MUST_BE_COMPLETE); PreconditionMessage.FILE_TO_PERSIST_MUST_BE_COMPLETE);
Expand All @@ -1953,9 +1953,8 @@ private void setStateInternal(long fileId, long opTimeMs, SetAttributeOptions op
* @param entry the entry to use * @param entry the entry to use
* @throws FileDoesNotExistException if the file does not exist * @throws FileDoesNotExistException if the file does not exist
*/ */
// TODO(calvin): Rename SetStateEntry to SetAttributeEntry, do not rely on client side options. private void setAttributeFromEntry(SetAttributeEntry entry) throws FileDoesNotExistException {
private void setStateFromEntry(SetStateEntry entry) throws FileDoesNotExistException { SetAttributeOptions.Builder options = new SetAttributeOptions.Builder();
SetAttributeOptions options = SetAttributeOptions.defaults();
if (entry.hasPinned()) { if (entry.hasPinned()) {
options.setPinned(entry.getPinned()); options.setPinned(entry.getPinned());
} }
Expand All @@ -1965,7 +1964,7 @@ private void setStateFromEntry(SetStateEntry entry) throws FileDoesNotExistExcep
if (entry.hasPersisted()) { if (entry.hasPersisted()) {
options.setPersisted(entry.getPersisted()); options.setPersisted(entry.getPersisted());
} }
setStateInternal(entry.getId(), entry.getOpTimeMs(), options); setAttributeInternal(entry.getId(), entry.getOpTimeMs(), options.build());
} }


/** /**
Expand Down
Expand Up @@ -24,14 +24,14 @@


import tachyon.Constants; import tachyon.Constants;
import tachyon.TachyonURI; import tachyon.TachyonURI;
import tachyon.client.file.options.SetAttributeOptions;
import tachyon.exception.FileDoesNotExistException; import tachyon.exception.FileDoesNotExistException;
import tachyon.exception.InvalidPathException; import tachyon.exception.InvalidPathException;
import tachyon.exception.TachyonException; import tachyon.exception.TachyonException;
import tachyon.master.file.options.CompleteFileOptions; import tachyon.master.file.options.CompleteFileOptions;
import tachyon.master.file.options.CreateDirectoryOptions; import tachyon.master.file.options.CreateDirectoryOptions;
import tachyon.master.file.options.CreateFileOptions; import tachyon.master.file.options.CreateFileOptions;
import tachyon.master.file.options.SetAclOptions; import tachyon.master.file.options.SetAclOptions;
import tachyon.master.file.options.SetAttributeOptions;
import tachyon.thrift.CompleteFileTOptions; import tachyon.thrift.CompleteFileTOptions;
import tachyon.thrift.CreateDirectoryTOptions; import tachyon.thrift.CreateDirectoryTOptions;
import tachyon.thrift.CreateFileTOptions; import tachyon.thrift.CreateFileTOptions;
Expand All @@ -47,7 +47,6 @@
* This class is a Thrift handler for file system master RPCs invoked by a Tachyon client. * This class is a Thrift handler for file system master RPCs invoked by a Tachyon client.
*/ */
@NotThreadSafe // TODO(jiri): make thread-safe (c.f. TACHYON-1664) @NotThreadSafe // TODO(jiri): make thread-safe (c.f. TACHYON-1664)
//TODO(dong): server side should use a separate SetAttributeOptions in tachyon.master.file.options
public final class FileSystemMasterClientServiceHandler implements public final class FileSystemMasterClientServiceHandler implements
FileSystemMasterClientService.Iface { FileSystemMasterClientService.Iface {
private final FileSystemMaster mFileSystemMaster; private final FileSystemMaster mFileSystemMaster;
Expand Down Expand Up @@ -231,8 +230,7 @@ public void scheduleAsyncPersist(String path) throws TachyonTException {
@Override @Override
public void setAttribute(String path, SetAttributeTOptions options) throws TachyonTException { public void setAttribute(String path, SetAttributeTOptions options) throws TachyonTException {
try { try {
mFileSystemMaster.setState(new TachyonURI(path), mFileSystemMaster.setAttribute(new TachyonURI(path), new SetAttributeOptions(options));
SetAttributeOptions.fromThriftOptions(options));
} catch (TachyonException e) { } catch (TachyonException e) {
throw e.toTachyonTException(); throw e.toTachyonTException();
} }
Expand Down
@@ -0,0 +1,161 @@
/*
* Licensed to the University of California, Berkeley under one or more contributor license
* agreements. See the NOTICE file distributed with this work for additional information regarding
* copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance with the License. You may obtain a
* copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/

package tachyon.master.file.options;

import javax.annotation.concurrent.NotThreadSafe;

import tachyon.conf.TachyonConf;
import tachyon.master.MasterContext;
import tachyon.thrift.SetAttributeTOptions;

/**
* Method option for setting the attributes.
*/
@NotThreadSafe
public class SetAttributeOptions {
/**
* Builder for {@link SetAttributeOptions}
*/
public static class Builder {
private Boolean mPinned;
private Long mTtl;
private Boolean mPersisted;
private long mOperationTimeMs;

/**
* Creates a new builder for {@link SetAttributeOptions}.
*/
public Builder() {
this(MasterContext.getConf());
}

/**
* Creates a new builder for {@link SetAttributeOptions}.
*
* @param conf a Tachyon configuration
*/
public Builder(TachyonConf conf) {
mPinned = null;
mTtl = null;
mPersisted = null;
mOperationTimeMs = System.currentTimeMillis();
}

/**
* @param pinned the pinned flag value to use
* @return the builder
*/
public Builder setPinned(boolean pinned) {
mPinned = pinned;
return this;
}

/**
* @param ttl the time-to-live (in seconds) to use
* @return the builder
*/
public Builder setTtl(long ttl) {
mTtl = ttl;
return this;
}

/**
* @param persisted the persisted flag value to use
* @return the builder
*/
public Builder setPersisted(boolean persisted) {
mPersisted = persisted;
return this;
}

/**
* @param operationTimeMs the operation time to use
* @return the builder
*/
public Builder setOperationTimeMs(long operationTimeMs) {
mOperationTimeMs = operationTimeMs;
return this;
}

/**
* Builds a new instance of {@link SetAttributeOptions}.
*
* @return a {@link SetAttributeOptions} instance
*/
public SetAttributeOptions build() {
return new SetAttributeOptions(this);
}
}

private final Boolean mPinned;
private final Long mTtl;
private final Boolean mPersisted;
private long mOperationTimeMs;

/**
* Constructs a new method option for setting the attributes.
*
* @param options the options for setting the attributes
*/
public SetAttributeOptions(SetAttributeTOptions options) {
mPinned = options.isSetPinned() ? options.isPinned() : null;
mTtl = options.isSetTtl() ? options.getTtl() : null;
mPersisted = options.isSetPersisted() ? options.isPersisted() : null;
mOperationTimeMs = System.currentTimeMillis();
}

/**
* @return the default {@link SetAttributeOptions}
*/
public static SetAttributeOptions defaults() {
return new Builder(MasterContext.getConf()).build();
}

private SetAttributeOptions(Builder builder) {
mPinned = builder.mPinned;
mTtl = builder.mTtl;
mPersisted = builder.mPersisted;
mOperationTimeMs = builder.mOperationTimeMs;
}

/**
* @return the pinned flag value
*/
public Boolean getPinned() {
return mPinned;
}

/**
* @return the time-to-live (in seconds)
*/
public Long getTtl() {
return mTtl;
}

/**
* @return the recursive flag value
*/
public Boolean getPersisted() {
return mPersisted;
}

/**
* @return the operation time
*/
public long getOperationTimeMs() {
return mOperationTimeMs;
}
}
Expand Up @@ -71,8 +71,8 @@ public static Message unwrap(JournalEntry entry) {
return entry.getRename(); return entry.getRename();
case SET_ACL: case SET_ACL:
return entry.getSetAcl(); return entry.getSetAcl();
case SET_STATE: case SET_ATTRIBUTE:
return entry.getSetState(); return entry.getSetAttribute();
case ENTRY_NOT_SET: case ENTRY_NOT_SET:
// This could mean that the field was never set, or it was set with a different version of // This could mean that the field was never set, or it was set with a different version of
// this message. Given the history of the JournalEntry protobuf message, the keys of the // this message. Given the history of the JournalEntry protobuf message, the keys of the
Expand Down

0 comments on commit 9f2f337

Please sign in to comment.