This is an automated email from the ASF dual-hosted git repository.
bharathkk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/samza.git
The following commit(s) were added to refs/heads/master by this push:
new cc21b6ebe Fix scope of caches for DirDiffUtil.areSameFile (#1671)
cc21b6ebe is described below
commit cc21b6ebebf829b8c7a47256d2717fc9250ba9c7
Author: Andy Sautins <git...@sautins.com>
AuthorDate: Thu Jun 15 10:24:41 2023 -0600
Fix scope of caches for DirDiffUtil.areSameFile (#1671)
Summary
Fix scope of owner and group name caches in DirDiffUtil.areSameFile
Detail
In #1669 the caches were scoped incorrectly and will be re-created each
time areSameFile called. Scoping the group and name caches outside of the
lambda to cache as expected.
Test
Added unit test TestDirDiffUtilAreSameFile.testAreSameFile_Cache to verify
the caches are working as expected.
./gradlew check
---
.../samza/storage/blobstore/util/DirDiffUtil.java | 11 ++--
.../blobstore/util/TestDirDiffUtilAreSameFile.java | 71 +++++++++++++++++++++-
2 files changed, 74 insertions(+), 8 deletions(-)
diff --git
a/samza-core/src/main/java/org/apache/samza/storage/blobstore/util/DirDiffUtil.java
b/samza-core/src/main/java/org/apache/samza/storage/blobstore/util/DirDiffUtil.java
index edae14645..6c2a70694 100644
---
a/samza-core/src/main/java/org/apache/samza/storage/blobstore/util/DirDiffUtil.java
+++
b/samza-core/src/main/java/org/apache/samza/storage/blobstore/util/DirDiffUtil.java
@@ -151,15 +151,16 @@ public class DirDiffUtil {
* @return BiPredicate to test similarity of local and remote files
*/
public static BiPredicate<File, FileIndex> areSameFile(boolean
compareLargeFileChecksums) {
+
+ // Cache owner/group names to reduce calls to
sun.nio.fs.UnixFileAttributes.group
+ Cache<String, String> groupCache =
CacheBuilder.newBuilder().maximumSize(CACHE_SIZE).build();
+ // Cache owner/group names to reduce calls to
sun.nio.fs.UnixFileAttributes.owner
+ Cache<String, String> ownerCache =
CacheBuilder.newBuilder().maximumSize(CACHE_SIZE).build();
+
return (localFile, remoteFile) -> {
if (localFile.getName().equals(remoteFile.getFileName())) {
FileMetadata remoteFileMetadata = remoteFile.getFileMetadata();
- // Cache owner/group names to reduce calls to
sun.nio.fs.UnixFileAttributes.group
- Cache<String, String> groupCache =
CacheBuilder.newBuilder().maximumSize(CACHE_SIZE).build();
- // Cache owner/group names to reduce calls to
sun.nio.fs.UnixFileAttributes.owner
- Cache<String, String> ownerCache =
CacheBuilder.newBuilder().maximumSize(CACHE_SIZE).build();
-
boolean areSameFiles;
PosixFileAttributes localFileAttrs;
try {
diff --git
a/samza-core/src/test/java/org/apache/samza/storage/blobstore/util/TestDirDiffUtilAreSameFile.java
b/samza-core/src/test/java/org/apache/samza/storage/blobstore/util/TestDirDiffUtilAreSameFile.java
index 15712f932..98f75122a 100644
---
a/samza-core/src/test/java/org/apache/samza/storage/blobstore/util/TestDirDiffUtilAreSameFile.java
+++
b/samza-core/src/test/java/org/apache/samza/storage/blobstore/util/TestDirDiffUtilAreSameFile.java
@@ -23,19 +23,27 @@ import java.io.BufferedWriter;
import java.io.File;
import java.io.FileWriter;
import java.nio.charset.StandardCharsets;
+import java.nio.file.FileSystem;
import java.nio.file.Files;
-import java.nio.file.attribute.PosixFileAttributes;
-import java.nio.file.attribute.PosixFilePermission;
-import java.nio.file.attribute.PosixFilePermissions;
+import java.nio.file.Path;
+import java.nio.file.attribute.*;
+import java.nio.file.spi.FileSystemProvider;
import java.util.ArrayList;
+import java.util.Map;
import java.util.Set;
import java.util.function.BiPredicate;
import java.util.zip.CRC32;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import junit.framework.Assert;
import org.apache.samza.storage.blobstore.index.FileIndex;
import org.apache.samza.storage.blobstore.index.FileMetadata;
import org.junit.Before;
import org.junit.Test;
+import org.mockito.Mockito;
+
+import static org.mockito.Mockito.*;
public class TestDirDiffUtilAreSameFile {
public static final int SMALL_FILE = 100;
@@ -183,4 +191,61 @@ public class TestDirDiffUtilAreSameFile {
remoteFile = new FileIndex(localFile.getName(), new ArrayList<>(),
remoteFileMetadata, localChecksum + 1);
Assert.assertTrue(areSameFile.test(localFile, remoteFile));
}
+
+ @Test
+ public void testAreSameFile_Cache() throws Exception {
+ createFile(LARGE_FILE);
+
+ for (int i = 0; i < 5; i++) {
+ BiPredicate<File, FileIndex> areSameFile =
DirDiffUtil.areSameFile(false);
+ for (int j = 0; j < 20; j++) {
+ localFile = mock(File.class);
+ when(localFile.getName()).thenReturn("name");
+
+ Path path = mock(Path.class);
+ when(localFile.toPath()).thenReturn(path);
+
+ FileSystem fileSystem = mock(FileSystem.class);
+ when(path.getFileSystem()).thenReturn(fileSystem);
+
+ FileSystemProvider fileSystemProvider = mock(FileSystemProvider.class);
+ PosixFileAttributes localFileAttributes =
mock(PosixFileAttributes.class);
+ when(localFileAttributes.size()).thenReturn(Long.valueOf(LARGE_FILE));
+
+ Map<String, Object> filePropMap = ImmutableMap.of("gid", j % 4,
+ "uid", j % 2);
+ when(fileSystemProvider.readAttributes(Mockito.any(Path.class),
+ Mockito.any(String.class),
Mockito.anyVararg())).thenReturn(filePropMap);
+ when(fileSystemProvider.readAttributes(Mockito.any(Path.class),
+ (Class<BasicFileAttributes>)
Mockito.any())).thenReturn(localFileAttributes);
+
+ UserPrincipal userPrincipal = mock(UserPrincipal.class);
+ when(userPrincipal.getName()).thenReturn("owner");
+
+ GroupPrincipal groupPrincipal = mock(GroupPrincipal.class);
+ when(groupPrincipal.getName()).thenReturn("group");
+
+ when(localFileAttributes.owner()).thenReturn(userPrincipal);
+ when(localFileAttributes.group()).thenReturn(groupPrincipal);
+
+ Set<PosixFilePermission> permissions = ImmutableSet.of();
+ when(localFileAttributes.permissions()).thenReturn(permissions);
+ when(fileSystem.provider()).thenReturn(fileSystemProvider);
+
+ remoteFile = mock(FileIndex.class);
+ when(remoteFile.getFileName()).thenReturn("name");
+ FileMetadata remoteFileMetadata = mock(FileMetadata.class);
+
when(remoteFileMetadata.getSize()).thenReturn(Long.valueOf(LARGE_FILE));
+ when(remoteFileMetadata.getGroup()).thenReturn("group");
+ when(remoteFileMetadata.getOwner()).thenReturn("owner");
+ when(remoteFileMetadata.getPermissions()).thenReturn("---------");
+ when(remoteFile.getFileMetadata()).thenReturn(remoteFileMetadata);
+
+ Assert.assertTrue(areSameFile.test(localFile, remoteFile));
+
+ Mockito.verify(localFileAttributes, times(j > 3 ? 0 : 1)).group();
+ Mockito.verify(localFileAttributes, times(j > 1 ? 0 : 1)).owner();
+ }
+ }
+ }
}
