Add FilePredicates for jobs #17525
Add FilePredicates for jobs #17525
Conversation
ssyssy
force-pushed
the
addPredicates
branch
2 times, most recently
from
7b2b659
to
17740e5
Compare
ssyssy
force-pushed
the
addPredicates
branch
5 times, most recently
from
bf92dae
to
0d02bf7
Compare
ssyssy
force-pushed
the
addPredicates
branch
3 times, most recently
from
02afc2b
to
873f1ab
Compare
| return predicate; | ||
| } | ||
| } | ||
| return new NoPredicate(); |
There was a problem hiding this comment.
I feel like the default semantic should be if no filter then all files should pass instead of no files pass
There was a problem hiding this comment.
@jja725 I think we should block the move job, because think about if user puts in an invalid filter, if we move all the files from source to destination. User would be frustrated to put everything back.
There was a problem hiding this comment.
In this case If they put a unknown filter, we should throw an exception then. And if they didn't put filter, we should have all things passed
| Iterable<FileInfo> fileIterator = | ||
| new UfsFileIterable(ufs, mJobEntry.getSrc(), user, mJobEntry.getPartialListing(), | ||
| FileInfo::isCompleted); | ||
| filePredicate.get()); |
There was a problem hiding this comment.
do you think we can combine create & get into one function?
There was a problem hiding this comment.
We could, but I think it would be more flexible if we have separate functions.
dora/core/server/master/src/main/java/alluxio/master/job/MoveJob.java
Outdated
Show resolved
Hide resolved
dora/core/server/master/src/test/java/alluxio/master/predicate/TimePredicateTest.java
Outdated
Show resolved
Hide resolved
dora/core/server/master/src/test/java/alluxio/master/predicate/TimePredicateTest.java
Outdated
Show resolved
Hide resolved
| @@ -116,7 +119,8 @@ public void testProgressReport() throws Exception { | |||
| FileIterable files = | |||
| new FileIterable(fileSystemMaster, srcPath, user, false, MoveJob.QUALIFIED_FILE_FILTER); | |||
| MoveJob job = spy(new MoveJob(srcPath, dstPath, false, user, "1", | |||
| OptionalLong.empty(), false, false, false, files)); | |||
| OptionalLong.empty(), false, false, false, files, | |||
| FileFilter.getDefaultInstance())); | |||
| when(job.getDurationInSec()).thenReturn(0L); | |||
| job.setJobState(JobState.RUNNING, false); | |||
| List<Route> nextRoutes = job.getNextRoutes(25); | |||
There was a problem hiding this comment.
do you mind adding some tests for the move job with predicate? To make sure we filter the current files. Also it's nice to cover what happened if we have an exception when findTimestampFromFileName
ssyssy
force-pushed
the
addPredicates
branch
2 times, most recently
from
55dab17
to
336c503
Compare
dora/core/server/master/src/main/java/alluxio/master/predicate/FilePredicate.java
Outdated
Show resolved
Hide resolved
dora/core/server/master/src/main/java/alluxio/master/predicate/FilePredicate.java
Outdated
Show resolved
Hide resolved
|
alluxio-bot, merge this please |
What changes are proposed in this pull request?
Adding the predicates for move jobs in order to be triggered by the policy engine.
Why are the changes needed?
Adding two predicates:
Does this PR introduce any user facing changes?
NA