Best way to test multiple bookings

[Samual Harvey]

What is the best approach to test the multiple clients doing search and booking. I have used RMI and have done a lot of testing using 2-3 users booking at the same time same flight number. Do I have to really create multiple threads to perform this test.
Thanks.

[Miguel Roque]

Hi.
I've also used RMI and i've created a small test program that randomly books seats. It accepts as parameter the number of users and creates one thread for each user. In the reservation method i've placed some code that performs a delay after the lock of the record of +/- 5sec's and the server gives information of the people waiting for the unlock.
I've tested with 1500 clients and it has worked fine . The server was my old PII 350MHz and the client was my double PIII 1000MHz.
You don't need to test 1500 clients . You create one class that creates lets say 20 clients and each of them will try to book the same flight and test it with flights with more that 20 seats and with flights with less than 20. Don't forget to put some of my prefered debug code 'System.out.println("I'm here");' to inform you of what it's happening.
Miguel

[John Smith]

What is the best approach to test the multiple clients doing search and booking. I have used RMI and have done a lot of testing using 2-3 users booking at the same time same flight number. Do I have to really create multiple threads to perform this test.

Create 20 threads, and have each thread book the same flight 200 times in a row. If at the end of the run the seat count for that flight is reduced by exactly 4000 seats, you passed.
Eugene.

[BJ Grau]

I did the same thing as Eugene, and I did an additional test where I let my threads attempt to overbook a flight. 15 threads woudl try to book 1 seat 10 times on a flight with 96 seats (for example), and when they are done running they would each report how many seats each <i>thought it had successfully booked</i>, and I verified that that total did not exceed the number of seats originally available.

[Samual Harvey]

OK, I have a question on this:-
What I tried is created 10 threads inside the client appliction which books the same flight.
Thus, inside the same client if I create 10/20 threads then I have to make sure that the methods in the client are synchronized. But I guess I can keep these methods as not synchronized because one client will perform the booking of flights as one request at a time. But by creating multiple threads the same client tries to book the same flight multiple times and if the methods performing the booking operations are not synchronized then it may lead to the abnormal results. But I dont think this is wrong as one client will do booking request one at a time. So do you guys think that doing multiple threads inside the client application is a good check, are you doing something else.
Now on my server side locking-unlocking is synchronized and I know if I keep an explicit lock on a flight number then the other clients requesting the same flight number wait until the first client releases the lock.
So if I use the synchronized methods that do the booking operations on the client side code and use 10 threads, the number of seats are reduced by 10. Is this correct? Should I keep the methods used by the client as synchronized versus just the locking-unlocking as synchronized?

[John Smith]

But by creating multiple threads the same client tries to book the same flight multiple times and if the methods performing the booking operations are not synchronized then it may lead to the abnormal results.

Your threads emulate multiple clients and they should call the same methods as your cients do. Do not synchronize these threads, -- they represent your clients on different machines. If this leads to "abnormal results", as you say, then your implementation is flawed, and the test uncovered it.
Eugene.

[Samual Harvey]

The problem I am facing is that at a given point two threads are getting the same available seats number and thus the results of 10 threads booking one seats are sometimes not 10.
Here is my lock code:-
I am using the sequence of:-
1.lock
2. get seats.
3. modify seats.
4. unlock
I am using a static HashMap as below:-

 
private static HashMap lockedRecords = new HashMap();
 

I also tried using following bu same problem:-

private static Map lockedRecords = Collections.synchronizedMap(new HashMap());

I tried a lot playing around with the code, but to no help. Can you all look at the following code and tell me if I am doing anything wrong.

  
public void lockClientRecord(int recordNumber, Object clientID) {
        synchronized(lockedRecords) {
            Integer record = new Integer(recordNumber);
                while ( lockedRecords.get(clientID) != null || lockEntireDb ) {
                    try {
                        lockedRecords.wait();
                    } catch(InterruptedException e) {
            }
                }
                lockedRecords.put(record, clientID);
        }
    }

Code for Unlock:-

  
public void unlockClientRecord(int recordNumber, Object clientID) {
        synchronized(lockedRecords) {
            Integer record = new Integer(recordNumber);
            if (!lockedRecords.isEmpty()) {
                if(lockedRecords.get(record) != null) {
             Object previousValue = 
                 lockedRecords.remove(record);
            }
                }
                lockedRecords.notifyAll();
            }
    }

[ March 13, 2003: Message edited by: Samual Harvey ]

[John Smith]

Can you all look at the following code and tell me if I am doing anything wrong.

You have several things that beg refactoring or are simply wrong:
1. The mapping should be from records to clients, not the other way around.
2. The method get() and remove() of the Map take a parameter that specifies the key of the object to get and remove. You are obvously using these methods wrong in your unlockClientRecord() method since the key in your map is clientID, while you are passing record number to the methods.
3. Instead of synchronized blocks, use synchronized methods, much easier to use and makes sense in these short methods.
4. For readability, replace "lockedRecords.get(yourKey) != null" with "lockedRecords.containsKey(yourKey)"
5. The call to lockedRecords.isEmpty() in your unlockClientRecord() method is redundant.
6. The object "previousValue" is never used and therefore should never be created.
7. What's the purpose of the lockedRecords.size() call, it's never used.
Too many problems in just a few lines of code, so I am not surprised it doesn't work.
Eugene.

[Samual Harvey]

1. I have mistyped here. I am actually using
lockedRecords.put(record, clientID)
2. So #2 should be correct.
3. I tried using synchronizing the complete method also but did not helped.
4. I will use this.
5. Yes I'll take this out.
6. Yes its not used just for debuggin so I'll take this out.
7. Thats again a typo.
I tried using all these and still the same problem. The way I am testing is I am creating multiple threads that do booking operation.
[ March 13, 2003: Message edited by: Samual Harvey ]

[John Smith]

I tried using all these and still the same problem

Post you lock/unlock code again, so that we are all confident that it's not where the problem is.
Eugene.

[Samual Harvey]

Somehow I am unable to call wait() and notifyAll() on lockedRecords as it gives me IllegalMonitorStateException, but if I use "this", it does not give the exception but the same problem.

  
private static Map lockedRecords = Collections.synchronizedMap(new HashMap());

    public synchronized void lockClientRecord(int recordNumber, Object clientID) {
            Integer record = new Integer(recordNumber);
                while ( lockedRecords.containsKey(record) || lockEntireDb ) {
                    try {
                        this.wait();
                    } catch(InterruptedException e) {
            }
                }
                lockedRecords.put(record, clientID);
    }
    
    public synchronized void unlockClientRecord(int recordNumber, Object clientID) {
            Integer record = new Integer(recordNumber);
            if(lockedRecords.get(record) != null) {
                   lockedRecords.remove(record);
            }
                this.notifyAll();
    }

A little update on the problem:- It is random, sometimes it does book the number of seats equal to the number of threads and sometimes it does not. Right now I ran this above code for 3 times using three threads, first 2 times it was ok but the third time it was not. For testing I have reduced the number of threads to be 3 at this time. The LockManager class for these methods is a singleton class.
If I put a sleep in the code between start of each thread then it works fine. I dont think putting a sleep here provides a true test. But I just thought of letting you know about it.
[ March 13, 2003: Message edited by: Samual Harvey ]

[John Smith]

You still have several problems:
1. Your lockedRecords collection doesn't need to be a synchronized map, since the access to it is granted from synchronized methods. This is more than just an additional level of synchronization that you don't need, -- it may actually cause deadlocks. Make you lock collection a simple HashMap.
2. You lockedRecords copllection should not be static, either. I don't want to get into the details of this in this thread, but you can find the explanation in other threads if you search for singleton.
3. Your unlockClientRecord() is simply wrong, -- the condition "if(lockedRecords.get(record) != null)" should be replaced with "if (lockedRecords.get(record).equals(clientID))"
4. The call notifyAll() should occur only if the record was unlocked.
Item 3) in this list explains explain why your locking doesn't work.
Eugene.

[Samual Harvey]

Thanks for a great help.
Like #3 should I change the lock logic to lock the record if the same record is locked by other client or should I keep the same where a new lock is not acquired if the same record is locked by the same clientID:-
(lockedRecords.containsKey(record) || lockEntireDb ) by
(!lockedRecords.containsKey(record).equals(clientID) || lockEntireDb )
An update, I wrote a test program to call the booking method multiple times from multiple threads and this seems to work good. I mean I ran 100 threads booking 1 seat each and I got the number of seats reduced by 100. I ran this test 4-5 times and is good. I also ran 20 threads each booking 20 seats and the ouput was good in the test program. The test program bypasses the GUI and directly calls the booking method. But when I create the threads inside my GUI to run the test from GUI, then I noticed that not all threads are reaching the booking method. I dont know why. Do you think I sould use the test program vs testing inside the GUI to test this.