Habibi book: How many instances of remote object and Data object exist ?

[Klaas van Gelder]

One question about the Habibi book (which i use to implement the FBN essay).

Ths dicussion on page 126-127 starts with a comparision bewteen sharing one DVDDatabase instance by all client versus one separate DVDDatabase instance for each client. They choose for the latter option, needing "a mechanism for coordination".
So each client has his own copy of the DVDDatabase object, and so of the DVDDbAdapter object and of the DVDDatabaseImpl object, because these objects contain exactly one instance of each other:

DVDDatabaseImpl <>-(1)----(1)-> DVDDbAdapter <>-(1)-------(1)-> Data

BUT:
In the server class (RegDVDDatabase), exactly one instance of the DVDDatabaseImpl is created and bound in the main method:

java.rmi.registry.LocateRegistry.createRegistry(1099);  //the default rmi port.
//make a dvd database instance
DVDDatabaseImpl aDVD = new DVDDatabaseImpl();
Naming.rebind("DVDMediator", aDVD);

which is looked up in the getRemote method of the DVDConnector class:

public static DBClient getRemote(String ip) throws RemoteException{
        try{
            //uncomment to register the DVDDatabase name programatically
            //RegDVDDatabase.register();
            return (DBClient)Naming.lookup("rmi://" + ip + "/DVDMediator");
        }
        catch(Exception e){
            throw new RemoteException(e.getMessage());
        }
 
    }

So every client looking up the remote object by the name "DVDMediator" receives the same instance of the remote object, and so of the Adapter en Data objects....
IMO this cannot be right. I have no real-life experience with RMI, but my feeling tells me that the remote object should spawn a thread for each new client.
So IMO the getRemote method should return an instance of some helper class having a method like: getDBClient(), which instantiates an Adapter class in a separate thread.... (am i getting lost here... ???)


By the way, Habibi says that he explicitly chooses to use one Data instance for each individual client. This also makes the locking mechanisem necessary. When only one shared DAta instance would be used, just synchronizing the access methods would be sufficient.

In the FBN essay, just as in the Habibi DVD project, the fact that the Data.lock and .unlock must be implemented, suggest IMO that more instances of the Data class exist at the same time.
But the Data class opens the db.db file, and only one db.db exists !
So my mind slowly adepts to the idea of one Data object shared by all clients.
But what is then the added value of the lock and unlock methods !!!???
If one and only one instance of the Data class exists, then synchronizing of the access methods already ensures that only one client at a time has access to the data... That makes the locking redundant... ???

Somehow I am loosing the track completely here !


Pleeze, who can help me to get on track again.... :-)
TIA, Klaas

[ July 21, 2004: Message edited by: Klaas van Gelder ]

[ July 21, 2004: Message edited by: Klaas van Gelder ]
[ July 21, 2004: Message edited by: Klaas van Gelder ]

[Klaas van Gelder]

In my FBNS essay, I now use a factory for creating a new instance of FlightDataImpl for each client. But one Data object is shared by all DataAdapter instances:

FlightDataImpl <>-(1)----(1)-> FlightDataAdapter <>-(N)-------(1)-> Data

This again raises the question why locking is necessary ! If there is onlyone Data instance and this is locked by a particular client (by entring one of the synchronized access methods), the other client threads have to waint anyway !?
I still miss someting... :-(

[Andrew Monkhouse]

Hi Klaas,

You are correct - Max's provided code only ever has one instance of DVDDatabase, while the book talks about having many instances of it, and coding for that potential. I think that the potential is the reason for the difference - the authors were trying to show what could be done in the future even though they did not code every possibility (they had to leave something for readers to do ).

But this really is secondary to your problem - understanding why we have synchronization and we have logical record locking.

Synchronization is required to ensure that only one thread is accessing the file at any given time. Or any other object for that matter.

But booking a record has a totally separate issue: ensuring that only one client books a particular seat (or seats). This requires your booking process to check that seats are still available to be booked, and then book them. Since there are two steps involved here, you need some way of making sure that no other thread can book your seats in between those two steps.

To make this a bit clearer, we are talking about two threads (A and B) both trying to book the same seat:

Thread A checks that the seat is still available

Thread B checks that the seat is still available

Thread A books the seat

Thread B books the seat

Logical record locking can help you avoid this problem - can you see how? (I don't want to just give the answer away).

Regards, Andrew
[ July 21, 2004: Message edited by: Andrew Monkhouse ]

[Klaas van Gelder]

Thanx Andrew !
I think i am getting your point.
The Data.modify method is synchronized and is an atomic operation.
But when thread A wants to book a number of seats, he first has to check whether those seats are available by calling the getRecord method.
Without locking it wound be possible that thread B books some seats inbetween thread A calls readRecord and modify.
In my current implementation of modify in the Adapter class, i do not check for those available seats:

  public void modify(DataInfo newData) throws IOException {
    int rec = newData.getRecordNumber();
    try {
      data.lock(rec, this);
      data.modify(newData);
      data.unlock(rec, this);
    }
    catch (DatabaseException ex) {
      throw new IOException(ex.getMessage());
    }
  }

So I guess that just before the call to modify, readRecord should be called followed by a check:

...
try {
  data.lock(rec, this);
  DataInfo oldData = data.getRecord(newData.getRecordNumber());
  //TODO: check seats
  data.modify(newData);
  data.unlock(rec, this);
}
...

And here occurs the following problem. How can the seats be checked ?
The argument of the modify method is a DataInfo object, not the number of
seats to book.
So without extending the Adapter class, the client must arrange this:
- The client selects a flight and enter the number of seats to book.
- The client modifies the DataInfo object for this flight and calls the Adapter.modify method.
-

Suppose the signature would be changed to the following (unchecked) code:

public void modify(DataInfo originalData, int seatsToBook) {
  int rec = originalData.getRecordNumber();
  data.lock(rec, this);
  DataInfo currentData = data.getRecord(rec);
  int currentSeats = Integer.parseInt(currentData.getValues()[AVAILABLE_SEATS]);
  if (currentSeats ) >= seatsToBook) {
    currentData.getValues()[AVAILABLE_SEATS] = "" + (currentSeats - seatsToBook);
    data.modify(currentData);
  }
  else {
    throw new XxxException("No seats available");
  }
  data.unlock(rec, this);
}

My apologies for the ugly and unverified code, but I hope that you grab my idea...
Because the number of seats to book is now explicitly passed in, the desired verification can be carried out.


The specs, however, clearly state that:
"The client program should include a class that implements the same public methods as the suncertify.db.Data class (...)"

Does this mean that we have to do it with the original methos signatures as the Data class ?
The code would also be much more readable if the DataInfo and FieldInfo classes would be wrapped by some "Facade" classes providing some more meaningful access methods. But again, is this a violation of the spec mentioned above ???
The same counts for the lock method. I pass the this parameter (a reference to the Adapter object) to caary out a check in the unlock method whether the rec was locked by the same client. But this also implies a signature change of the lock method...
The specs, however, do not tell explicitly that the Data method signatures may not be changed or that new methods may not be added...
The specs DO tell us:
"You are required to implement the criteriaFind(String), lock(int) and unlock(int) methods". Should we interpret this so strictly that no additional parameters may be added ???

I am still fighting with this issue.... Any more suggestions are more than welcome.
Regards, Klaas

[ July 21, 2004: Message edited by: Klaas van Gelder ]
[ July 21, 2004: Message edited by: Klaas van Gelder ]

[Klaas van Gelder]

I mean, the DataInfo class could be wrapped as below to make the code more readable...

package suncertify.db;
 
public class RecordFacade {
  private DataInfo dataInfo;
  
  public RecordFacade(DataInfo dataInfo) {
    this.dataInfo = dataInfo;
  }
  
  public String getFlightNumber() {
    return dataInfo.getValues()[0];
  }
  
  public String getOriginalAirport() {
    return dataInfo.getValues()[1];
  }
  
  public String getDestinationAirport() {
    return dataInfo.getValues()[2];
  }
  
  public String getCarrier() {
    return dataInfo.getValues()[3];
  }
  
  public String getPrice() {
    return dataInfo.getValues()[4];
  }
  
  public String getDay() {
    return dataInfo.getValues()[5];
  }
  
  public String getTime() {
    return dataInfo.getValues()[6];
  }
  
  public String getDuration() {
    return dataInfo.getValues()[7];
  }
  
  public int getAvailableSeats() {
    return Integer.parseInt(dataInfo.getValues()[8]);
  }
  
  //setters
}

[Andrew Monkhouse]

Hi Klaas,

The specs, however, clearly state that:
"The client program should include a class that implements the same public methods as the suncertify.db.Data class (...)"

The way I handled that requirement was to have a Facade on the client side which called either the RMI methods of the same name, or the direct connection methods of the same name. This Facade hid the method of connecting to the database. Then I had my bookSeats() method call the relevant lock() getRecord() modify() etc methods from the Facade.

Regards, Andrew

[Klaas van Gelder]

Hi Andrew,
I am afrain i do not exactly understand what you mean. You talk about a facade on the client side, so I guess you use the original methods in the Data class and the Adapter class (which calls the lock methods).

My idea is to formulate some new public methods for the Data class:
(I renamed REcordFacade to RecordAdapter, because this is more an example of the Adapter pattern than of the Facade pattern)
public synchronized RecordAdapter getFlightRecord(int rec) throws DatabaseException
public synchronized RecordAdapter findFlightRecord(String toMatch) throws DatabaseException
public synchronized void deleteFlightRecord(int rec) throws DatabaseException
public synchronized void modifyFlightRecord(RecordAdapter record) throws DatabaseException

Those methods then form the new "public interface" of the Data class (again, there is no spec telling us that the orinal public methods may not be altered)
The orginial methods are changed to private and are called from the new methods mentioned above.
Only for the lock and unlock methods, the specs tell us to implement them with the original signature. But without adding an argument, I don't see how to track client info.

These methodes are mirrored in the FlightDataServices interface, implemented by the FlightDataAdapter class and by the remote object.
The FlightDataServices interface also defines the method bookSeats, which is implemented in FlightDataAdapter:

  public void bookSeats(int recNum, int seatsToBook) throws IOException {
    try {
      data.lock(recNum, this);
      RecordFacade currentRecord = data.getFlightRecord(recNum);
      int seatsAvailable = currentRecord.getAvailableSeats();
      if (seatsAvailable >= seatsToBook) {
        currentRecord.setAvailableSeats(seatsAvailable - seatsToBook);
        data.modifyFlightRecord(currentRecord);
      }
      else {
        //TODO: doe iets
      }
      data.unlock(recNum, this);
    }
    catch(DatabaseException dbe) {
      throw new IOException(dbe.getMessage());
    }
  }

I think this method is the better one, in fact bookSeats is a business method and does not actually belong to the Data class...
but now it makes no sense to use the original Data methods on the client side !!
So to summarize, the issues are:
- May the public interface Data class be modified ?
- Is it a good idea to wrap the DataInfo class in a RecordAdapter class ?
- Am I right in my implemetnation of the bookSeats method ?

I look forward to your feedback... :-)
Regards, Klaas

[ July 22, 2004: Message edited by: Klaas van Gelder ]

[ July 22, 2004: Message edited by: Klaas van Gelder ]
[ July 22, 2004: Message edited by: Klaas van Gelder ]

[Satish Avadhanam]

Originally posted by Klaas van Gelder:
Hi Andrew,
I am afrain i do not exactly understand what you mean. You talk about a facade on the client side, so I guess you use the original methods in the Data class and the Adapter class (which calls the lock methods).

I think that is what Andrew meant. If its a remote client, it goes through RMI metthods, if its local it directly calls Data class methods. Please correct me if I'm wrong? Thanks.

Those methods then form the new "public interface" of the Data class (again, there is no spec telling us that the orinal public methods may not be altered)

Its not advisable to change given public interface. There are many threads on this topic whether to change or not, but many people stick with not modifying given interface.


So to summarize, the issues are:
- May the public interface Data class be modified ?

As said above, not adivsable

[Klaas van Gelder]

Thanx Satish, I think you are right and it is better to leave the public interface of the Data class unchanged.
I still maintain my RecordAdapter class for wrapping the DataInfo objects, but now I have moved this action to the DataAdapter class.

Again, my reason to use this RecordAdapter is to provide an interface to a record containing readable and convenient access methods, for example for retrieving the number of booked seats as an int value.

The problem is that the specs require us, as quoted before:

"The client program should include a class that implements the same public methods as the suncertify.db.Data class (...)"

That could be an argument for not using a record wrapper-class at all. But then, access to all fields must occur via the DataInfo.values[] field, passing the right field index and, if necessary, converting the String value to an int. Introducing constants for the index values in the DataInfo class would already be an improvement...
Or maybe we could only use the wrapper class INSIDE the public methods of the DataAdapter class (and later on the client), but leave the public interface the same as of the Data class...
Greetz, Klaas
[ July 23, 2004: Message edited by: Klaas van Gelder ]