0
\$\begingroup\$

I have a remote hosted server that's running Java VM with custom server code for a multiplayer real-time quiz game. The server deals with matchmaking, rooms, lobbies etc. I'm also using a MongoDB on same space which holds all the questions for the mobile phone quiz game.

This is my first attempt at such a project and although I'm competent in Java and my JSON and Mongo skill are novice.

Objective

I hope someone would take a look at the following game room code which queries the db for questions holds a copy of them for broadcasting to the clients during the quiz. I'm hoping someone will advise on how I can improve or even suggest a more elegant solution for this, especially the way I manage the connection between the server and MongoDB including my db schema.

Information

The quiz is based on popular TV shows and the player has an option to drill down into the show to choose a category that the would like to pay. For example, if they choose Game of Thrones, then they have the option to play a sub-level topic quiz such as one of the leading characters from the show or even season 1 etc.

The questions for the TV shows have meta data attached to them. For example, Game of Thrones questions relating to the character Aray Stark has "Aray Stark" meta data attached to the question. The client passes this string to the server during a connection request and the server moves the player to a room and passes along the string. The room queries the db for this meta data string, pulls back questions and fires these to clients during game play.

Client Singleton

My server contains a static singleton of Mongo clients:

     public class ClientSingleton 
  {

  private static ClientSingleton uniqueInstance;
  // The MongoClient class is designed to be thread safe and shared among threads. 
  // We create only 1 instance for our given database cluster and use it across
  // our application.
  private MongoClient mongoClient;
  private MongoClientOptions options;
  private MongoCredential credential;

  private final String password = "xxxxxxxxxxxxxx";
  private final String host = "xx.xx.xx.xx";
  private final int port = 38180;

  /**
    * 
  */
  private ClientSingleton() 
{
    // Setup client credentials for DB connection (user, db name & password)
    credential =  MongoCredential.createCredential("XXXXXX", "DBName", password.toCharArray());
    options = MongoClientOptions.builder()
            .connectTimeout(25000)
            .socketTimeout(60000)
            .connectionsPerHost(100)
            .threadsAllowedToBlockForConnectionMultiplier(5)
            .build();
    try 
    {
        // Create client (server address(host,port), credential, options)
        mongoClient = new MongoClient(new ServerAddress(host, port), 
                Collections.singletonList(credential),
                options);
    } 
    catch (UnknownHostException e) 
    {
        e.printStackTrace();
    }
}

  /**
   * Double checked dispatch method to initialise our client singleton class
   * 
   */
  public static ClientSingleton getInstance()
  {
    if(uniqueInstance == null)
    {
        synchronized (ClientSingleton.class)
        {
            if(uniqueInstance == null)
            {
                uniqueInstance = new ClientSingleton();
            }
        }
    }
    return uniqueInstance;
  }

  /**
   * @return our mongo client
   */
  public MongoClient getClient() {
    return mongoClient;
  }
 }

Notes:

Mongo client is new to me and I understand failure to properly utilise connection-pooling is one major "gotcha" that greatly impact MongoDB performance. Creating new connections to the db is also expensive and I should try and re-use existing connections.

I've not left socket timeout and connect timeout at defaults (e.g. infinite) if connection hangs for some reason I think it will get stuck forever! I set the number of milliseconds the driver will wait before a connection attempt is aborted, for connections made through a Platform-as-a-Service (where the server is hosted) it is advised to have a higher timeout (e.g. 25 seconds). I also set the number of milliseconds the driver will wait for a response from the server for all types of requests (queries, writes, commands, authentication, etc.). Finally I set threadsAllowedToBlockForConnectionMultiplier to 5 (500) connection in, a FIFO stack, awaiting their turn on the db.

Server Zone

Zone gets a game request from the client and receives the meta data string for the quiz type. In this case, "Episode 3". Zone creates a room for the user or allows the user to join the room with with that property.

Server Room

The room then establishes a db connection to a Mongo collection for the TV show:

// Get client & collection
mongoDatabase = ClientSingleton.getInstance().getClient().getDB("DBName");
mongoColl = mongoDatabase.getCollection("GOT");

// Query mongo db with meta data string request
queryMetaTags("Episode 3");

Notes:

Following a game or I should say after an room idle time the room get destroyed - this idle time is currently set to 60 mins. I believe that if connections per host is set to 100, then while this room is idle, it would be using valuable connection resources. If I have several hundred concurrently connected games and each accessing the db to pull the questions, then maybe following that request free up the client connection for other rooms to use? How should this be done? I'm concerned about possible bottlenecks here!

The room then queries the TV show collection and searches through its documents for questions matching our search string: (Note: I was not the author of this query - this was suggested by Yathish Manjunath on Stack Overflow, so do not review it.)

Mongo Query

    // Query our collection documents metaTag elements for a matching string
// @SuppressWarnings("deprecation")   public void queryMetaTags(String query)
{         // Query to search all documents in current collection
    List<String> continentList = Arrays.asList(new String[]{query});
    DBObject matchFields = new 
       BasicDBObject("season.questions.questionEntry.metaTags", 
      new BasicDBObject("$in", continentList));
    DBObject groupFields = new BasicDBObject( "_id", "$_id").append("questions", 
       new BasicDBObject("$push","$season.questions"));
    //DBObject unwindshow = new BasicDBObject("$unwind","$show");
    DBObject unwindsea = new BasicDBObject("$unwind", "$season");
    DBObject unwindepi = new BasicDBObject("$unwind", "$season.questions");
    DBObject match = new BasicDBObject("$match", matchFields);
    DBObject group = new BasicDBObject("$group", groupFields); 
    @SuppressWarnings("deprecation")      AggregationOutput output = 
    mongoColl.aggregate(unwindsea,unwindepi,match,group);

    String jsonString = null;
    JSONObject jsonObject = null;
    JSONArray jsonArray = null;
    ArrayList<JSONObject> ourResultsArray = new ArrayList<JSONObject>();

    // Loop for each document in our collection
    for (DBObject result : output.results()) 
    {         
        try 
        {
          // Parse our results so we can add them to an ArrayList
          jsonString = JSON.serialize(result);             
          jsonObject = new JSONObject(jsonString);
          jsonArray = jsonObject.getJSONArray("questions");

          for (int i = 0; i < jsonArray.length(); i++)
          {
              // Put each of our returned questionEntry elements into an ArrayList
              ourResultsArray.add(jsonArray.getJSONObject(i));
          }                
      } 
        catch (JSONException e1) 
        {
          e1.printStackTrace();
      }
    }   
  pullOut10Questions(ourResultsArray);
}

Notes:

For challenge games, this query works well to access the documents searching for metaTag elements in my JSON example for strings that match our game request type. If the user wants to play chronological mode then I will have to create another query that searches the JSON document in the TV show collection and pull out all questions for that episode. The element in the JSON examples I give below is named "questions" (e.g. episode) that sits inside the season element. Indexing is another issue I'm unsure how best to do this for this setup.

Query Results

The result pulls back every questionEntry element in the document that has a metaTag array element which match the search string.

Each game match consists of 10 questions, so we pull out a random 10 from our results:

    public void pullOut10Questions(ArrayList<JSONObject> ourResultsArray)
{
    // Array to hold 10 random numbers between 0 and our results total
    ArrayList<Integer> ourRandomNumbersList = generate10RandomNumbersInRange(ourResultsArray.size());
    // Array to hold our 10 random questions from our results
    ArrayList<JSONObject> ourQuestionsArray = new ArrayList<JSONObject>();

    // Loop through each of our results in array
    for (int i = 0; i < ourResultsArray.size(); i++)
    {
        // Loop through our array holding our 10 random numbers
        for(int j = 0; j < ourRandomNumbersList.size(); j++)
        {
            // If our results array index equals one of our 10 random numbers
            if(ourRandomNumbersList.get(j) == i)    
                // Then add that result to our final questionElement array
                ourQuestionsArray.add(ourResultsArray.get(i));
        }
    }
}

// Return 10 random numbers in range
public ArrayList<Integer> generate10RandomNumbersInRange(int range)
{
    Random rand = new Random();
    int e;
    int i;
    int g = 10;
    // Store random numbers is HashSet
    HashSet<Integer> randomNumbers = new HashSet<Integer>();

    for (i = 0; i < g; i++) 
    {
        e = rand.nextInt(range);
        randomNumbers.add(e);
        // Keep adding numbers until we reach 10
        if (randomNumbers.size() <= 10) 
        {
            if (randomNumbers.size() == 10) 
                g = 10;
            g++;
            randomNumbers.add(e);
        }
    }
    // Return our random numbers as an ArrayList
    ArrayList<Integer> al = new ArrayList<Integer>();
    Iterator<Integer> iter = randomNumbers.iterator();
    while(iter.hasNext())
    {
        al.add(iter.next());
    }
    return al;
}

Each TV show has its own collection on the db and the collection consists of JSON documents that contain the questions for each of the TV show seasons. Here are two sample documents for the Game of Thrones TV show:

JSON Examples

The room then broadcasts these questions simultaneously to each client, totaling up their scores and announcing the winner!

Notes:

I decided on this schema for the JSON and Mongo collections because the player also has the option to play solo in chronological mode for the TV show. For example, the player starts in season 1 episode 1 for that show and works their way through the entire show answering questions. So each JSON document holds the questions for that series and all its episodes in chronological order.

At the moment I'm extracting the question string and broadcasting that along with the last answer question to the clients - this is pretty much as far as I've got. I have to say having played with JSON in Java I'm thinking of using POCO's (jsonschema2pojo) for the first time to make handling JSON question data easily. In C#, which is what I'm used to, JSON is handled easily by injecting it into objects etc. using JSON to C#.

\$\endgroup\$
2
  • \$\begingroup\$ Well... I don't want to write an answer just for this one point but your getInstance is still not threadsafe, you need to check for == null in your synchronized-block a second time and i wouldn't use volatile, its a unnecessary slowdown, synchronized flushes cache anyways. \$\endgroup\$
    – tkausl
    Aug 6, 2015 at 9:52
  • \$\begingroup\$ @tkausl Thanks I totally missed that one. Would volatile here ensure that multiple threads handle the uniqueInstance variable correctly when it is being initiated to the singleton instance? \$\endgroup\$ Aug 6, 2015 at 9:57

1 Answer 1

1
\$\begingroup\$

Since the text i wrote was a little bit long for a comment i post it as answer.

Okay, back to the beginning:

  public static ClientSingleton getInstance()
  {
    if(uniqueInstance == null)
    {
        synchronized (ClientSingleton.class)
        {
            if(uniqueInstance == null)
            {
                uniqueInstance = new ClientSingleton();
            }
        }
    }
    return uniqueInstance;
  }

You'll definitely need to check the instance a second time in your synchronized since two threads could simultaneously pass the if and both - even though they run one after the other through the synchronzied - create a new instance. You've done this second check now and edited this change into your question, thats good.

Now to your volatile. Volatile essentially prevents any caching-mechanism. Thats actually not what you want here. After the Instance has been created it will never ever change again - its a singleton - and volatile is really expensive if you compare it with a cache-hit. In this case, you don't even need the volatile since, as i said in one of my comments, synchronized flushes the cache implicitly. This means, in the worst case scenario, if some thread has still an old value (this means null) in the cache, it will pass the first if, acquire the lock, fail the second if since the cache has been flushed and it now has the correct value, and from this moment on, will never again pass the first if since it may cache the - now correct - singleton-instance forever.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the explanation there. I have updated my post to reflect your recommended change. \$\endgroup\$ Aug 6, 2015 at 10:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.