Skip to content
Advertisement

Executor service returning incorrect response

I am creating future list from a list of calls to executor service submit method based on student ID. The response from service is not returning for all studentId’s. It runs for the right number of times but the studentId getting used in service call is either first or last. It is ignoring the middle ones. Please check the code below

private List<Student> studentTask() {
        ExecutorService executor = Executors.newFixedThreadPool(5);
        List<Future<List<Student>>> tasks = new ArrayList<>();
        for(int studentNumber = 1; studentNumber <= 10; studentNumber++) {
            Callable<List<Student>> task = new StudentService(studentNumber);
            Future<List<Student>> recordTask = executor.submit(task);
            tasks.add(recordTask);
        }
        try {
            for (Future<List<Student>> future : tasks) {
                List<Student> student = future.get();
                //...
            }
            return //studentList;
        } catch (Exception e) {
        }
    }

private class StudentService implements Callable<List<Student>>{
   private int studentId;
    
   StudentService(int studentId){
     this.studentId = studentId;
   }
    
   public List<Student> call(){
     return getStudentNames(this.studentId);
   }
 }

public class Student{
   private String studentName;
   private int StudentId;

   //Parameterized Constructor
}

private List<Student> getStudentNames(int studentId){
   List<Student> studentList = // db call returning all student with 
                               // respect to studentId.
   return studentList;
}

In the below code the service is getting called 10 times but for only student Id 1 and 10. Not able to get result of 2 to 9 which is resulting in an inaccurate result. Need help in understanding if i am missing anything here.

Advertisement

Answer

Your code is faulty to the point of not compiling, as noted in my Comment. And you’ve omitted some possibly important code. So I cannot diagnose exactly what problems you have. So I will revamp your code in my own style, and get it working.

Rather than trying to fill a list of Future objects, I would fill a list of tasks, either Runnable or Callable objects. You can then call ExecutorService#invokeAll to submit all the tasks. You get back a list of Future objects to track the completion of your submitted tasks.

First, let’s define the Student class as a record.

record Student( int number , String name ) { }

It seems to me you have mixed two different responsibilities into the StudentService class. That class should focus only on holding the students’s data. That class should not be the Callable. Define the Callable separately, and pass the StudentService object to its constructor.

Notice that we return an Optional. If the calling programmer provides an invalid student ID, we return an empty Optional rather than a null pointer.

class StudentService
{
    private Set < Student > students;

    StudentService ( )
    {
        this.students =
                Set.of(
                        new Student( 1 , "Alice" ) ,
                        new Student( 2 , "Bob" ) ,
                        new Student( 3 , "Carol" ) ,
                        new Student( 4 , "Davis" ) ,
                        new Student( 5 , "Ernestine" ) ,
                        new Student( 6 , "Frank" ) ,
                        new Student( 7 , "Gail" ) ,
                        new Student( 8 , "Harold" ) ,
                        new Student( 9 , "Iris" ) ,
                        new Student( 10 , "Jean-Luc" )
                );
    }

    synchronized Optional < Student > fetchStudentById ( final int id )
    {
        return this.students.stream().filter( student -> student.id() == id ).findAny();
    }
}

Notice in code above that the fetchStudentById is marked synchronized. We know this method will be invoked across threads. The current implementation here may be thread-safe, by streaming over a non-modifiable List. But in real work, this look-up may not be thread-safe. So we mark it synchronized for thread-safety.

If you are not comfortable with streams as seen in that code above, know that you could accomplish the same effect with a conventional loop. Using streams makes for briefer code, but the use of streams here is not important.

Define our task, a Callable that looks up a student by ID, and returns a Optional < Student >. We pass to its constructor the StudentService object to be used to actually find the student. And we pass the id of the desired student.

class StudentFindingTask implements Callable < Optional < Student > >
{
    private final StudentService studentService;
    private final int studentId;

    public StudentFindingTask ( final StudentService studentService , int studentId )
    {
        this.studentService = studentService;
        this.studentId = studentId;
    }

    @Override
    public Optional < Student > call ( ) throws Exception
    {
        return this.studentService.fetchStudentById( this.studentId );
    }
}

Now we are ready to try this out.

Instantiate a StudentService object to be used by all our tasks.

StudentService studentService = new StudentService();

Establish a list of task objects. Pass the service and id to each.

int limit = 10;
List < StudentFindingTask > tasks = new ArrayList <>( limit );
for ( int studentId = 1 ; studentId <= limit ; studentId++ )
{
    tasks.add( new StudentFindingTask( studentService , studentId ) );
}

Prepare the executor service, and the list of Future objects we expect it to fill.

ExecutorService executorService = Executors.newFixedThreadPool( 5 );
List < Future < Optional < Student > > > futures;

Submit all those tasks to the executor service.

try { futures = executorService.invokeAll( tasks ); } catch ( InterruptedException e ) { throw new RuntimeException( e ); }

Shutdown the executor service, and wait for results. The following is boilerplate code taken from Javadoc, and slightly modified.

executorService.shutdown(); // Stop accepting task submissions.
try
{
    if ( ! executorService.awaitTermination( 60 , TimeUnit.SECONDS ) )
    {
        executorService.shutdownNow(); // Cancel currently executing tasks
        if ( ! executorService.awaitTermination( 60 , TimeUnit.SECONDS ) )
        { System.err.println( "Pool did not terminate" ); }
    }
}
catch ( InterruptedException ex )
{
    executorService.shutdownNow();
    Thread.currentThread().interrupt();
}

Lastly, report the results of our tasks by examining each Future object. In real work, you would likely interrogate the future for its completion status, but I’ll leave that as an exercise for the reader.

for ( Future < Optional < Student > > future : futures )
{
    try { System.out.println( future.get() ); } catch ( InterruptedException e ) { throw new RuntimeException( e ); } catch ( ExecutionException e ) { throw new RuntimeException( e ); }
}

When run.

Optional[Student[id=1, name=Alice]]
Optional[Student[id=2, name=Bob]]
Optional[Student[id=3, name=Carol]]
Optional[Student[id=4, name=Davis]]
Optional[Student[id=5, name=Ernestine]]
Optional[Student[id=6, name=Frank]]
Optional[Student[id=7, name=Gail]]
Optional[Student[id=8, name=Harold]]
Optional[Student[id=9, name=Iris]]
Optional[Student[id=10, name=Jean-Luc]]
Advertisement