Skip to content
This repository has been archived by the owner on Jan 3, 2023. It is now read-only.

Merge SSM Compression to trunk #1873

Merged
merged 133 commits into from
Jan 28, 2019
Merged

Merge SSM Compression to trunk #1873

merged 133 commits into from
Jan 28, 2019

Conversation

qiyuangong
Copy link
Contributor

No description provided.

timmyyao and others added 30 commits October 30, 2017 15:09
@qiyuangong qiyuangong changed the title [Do not merge] Test merge Compression to trunk Merge SSM Compression to trunk Jan 16, 2019
@qiyuangong
Copy link
Contributor Author

@PHILO-HE This PR has passed all building checks and UTs. Pls review it ASAP, such that we can merge it with trunk.

@PHILO-HE
Copy link
Member

SSM docs should be updated, e.g., updating web help page for action, adding a design doc under SSM/doc.

public static final String SMART_COMPRESSION_BUFFER_SIZE = "smart.compression.buffer.size";
public static final int SMART_COMPRESSION_BUFFER_SIZE_DEFAULT = 262144;
public static final String SMART_COMPRESSION_MAX_SPLIT = "smart.compression.max.split";
public static final int SMART_COMPRESSION_MAX_SPLIT_DEFAULT = 1000;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The config file smart-default.xml should be updated accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added smart.compression.impl into smart-default.

if (srcFile.getLen() == 0) {
compressionFileInfo = new CompressionFileInfo(false, compressionFileState);
} else {
String tempPath = "/tmp/ssm" + filePath + "." + System.currentTimeMillis() + ".ssm_compression";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep consistent with EC, we can set the temp dir as /system/ssm/compress_tmp in the scheduler. Then in the scheduler instead of action, the tempPath is set as TEMP_DIR/ + fileName + "" + "aid" + action.getActionId() + "" + System.currentTimeMillis(). Using filePath is not proper. Adding aid is friendly to debug.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refined this part according to comments.


public void setDfsClient(DFSClient dfsClient) {
this.dfsClient = dfsClient;
}

public void setDefaultDfsClient(DFSClient dfsClient) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that creating defaultDfsClient is unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

gson.toJson(compressionInfo.getCompressedPos()));
}

public void deleteByName(String fileName) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deleteByPath(String filePath)

new CompressFileRowMapper());
}

public CompressionFileState getInfoByName(String fileName) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getInfoByPath(String filePath)

+ " type tinyint(4) NOT NULL,\n"
+ " stage tinyint(4) NOT NULL\n"
+ ");",
"CREATE TABLE compression_file (\n"
+ " file_name varchar(512) PRIMARY KEY,\n"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file_name -> path

@@ -179,6 +187,16 @@ public FileStatus getFileStatus(Path f) throws IOException {
oldStatus.getOwner(), oldStatus.getGroup(),
oldStatus.isSymlink() ? oldStatus.getSymlink() : null, oldStatus.getPath());
}
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if oldStatus=null? This else code block will throw NPE when oldStatus=null.

@@ -171,6 +185,16 @@ public FileStatus getFileStatus(Path f) throws IOException {
oldStatus.getOwner(), oldStatus.getGroup(),
oldStatus.isSymlink() ? oldStatus.getSymlink() : null, oldStatus.getPath());
}
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if oldStatus=null? This else code block will throw NPE when oldStatus=null.

@@ -171,6 +190,16 @@ public FileStatus getFileStatus(Path f) throws IOException {
oldStatus.getOwner(), oldStatus.getGroup(),
oldStatus.isSymlink() ? oldStatus.getSymlink() : null, oldStatus.getPath());
}
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if oldStatus=null? This else code block will throw NPE when oldStatus=null.

@PHILO-HE
Copy link
Member

Please check how compression action react for a directory arg.

@qiyuangong qiyuangong merged commit 85db2e5 into trunk Jan 28, 2019
@qiyuangong qiyuangong deleted the compression-rebase branch January 28, 2019 14:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants