0

I'm writing a server-side program that logs events on a per-user basis. I have the following code to do this :

public void logUser(long uniqueID, String event) throws IOException
{
    BufferedWriter buffWriter = new BufferedWriter(new FileWriter(uniqueID + ".log", true));
    buffWriter.write(event);
    buffWriter.close();
}

Now as multiple different events may be called on the same user from different threads in a small difference of time, I'd like to synchronize this process on a per-user basis. This is because the files used are unique per user.

I'd like to know the most efficient way to achieve this.

Raedwald
  • 40,290
  • 35
  • 127
  • 207
Hele
  • 1,409
  • 4
  • 18
  • 33
  • 4
    Use a log framework perhaps? – Magnilex Jan 02 '15 at 19:03
  • I'd rather stick to standard Java unless absolutely necessary. – Hele Jan 02 '15 at 19:04
  • Java has a standard logging interface. Logging is just not one of those things you should attempt to reinvent unless you a) want to learn how to write a logging framework or b) have some exceptionally unique requirements (which you do not appear to have). – Paolo Jan 02 '15 at 19:18
  • @Hele not using a logging framework when this is exactly what you need is going to be painful. – wha'eve' Jan 02 '15 at 19:18
  • Did not know that. Could you perhaps point me to some resource which could serve my needs? – Hele Jan 02 '15 at 19:19
  • @Paolo Perhaps you meant _not_ attempt to reinvent? – Hele Jan 02 '15 at 19:22
  • Can u be more specific : – Manish Jan 02 '15 at 19:23
  • More specific? I dont understand.. – Hele Jan 02 '15 at 19:23
  • Just synchronize the method. If you don't wanna lock using the current instance, create your own lock instance to use – Dioxin Jan 02 '15 at 19:23
  • @VinceEmigh That would cause a major performance bottleneck and is against the point of my question. – Hele Jan 02 '15 at 19:24
  • Javadoc is here http://docs.oracle.com/javase/8/docs/api/java/util/logging/package-summary.html but I would strongly recommend looking at SLF4J and Logback instead of the standard library. – Paolo Jan 02 '15 at 19:25
  • 1
    Your code will leak resources if the `write` throws an exception. – Raedwald Jan 02 '15 at 19:27

3 Answers3

4

I can't recommend rewriting a logging framework because it is much more complicated than it sounds and you would almost certainly be better off using an existing one. See for example Java Logging vs Log4J and Why not use java.util.logging? for various alternatives.

However for your use case you could use a ConcurrentHashMap of locks:

ConcurrentMap<Long, Object> locks = new ConcurrentHashMap<> ();

public void logUser(long uniqueID, String event) throws IOException {
  Object lock = locks.computeIfAbsent(uniqueID, i -> new Object());
  synchronized(lock) {
    try (BufferedWriter buffWriter =
                 new BufferedWriter(new FileWriter(uniqueID + ".log", true));) {
      buffWriter.write(event);
    }
  }
}   
Community
  • 1
  • 1
wha'eve'
  • 3,010
  • 2
  • 9
  • 6
2

I believe an alternative to synchronizing the whole thing is to use an async approach. Let's say that all of the log entries are added to a BlockingQueue and some other thread consumes the queue. Then, no synchronization is needed.

Example:

public class LogAsync {
    // Some kind of abstraction for a log entry
    public static class LogEntry {
        private final String event;
        private final long uniqueId;

        public LogEntry(long uniqueId, String event) {
            this.uniqueId = uniqueId;
            this.event = event;
        }

        public String getEvent() {
            return event;
        }

        public long getUniqueId() {
            return uniqueId;
        }
    }

    // A blocking queue where the entries are stored    
    private final BlockingQueue<LogEntry> logEvents = new LinkedBlockingQueue<>();

    // Adds a log entry to the blocking queue    
    public void logUser(long uniqueID, String event) {
        logEvents.add(new LogEntry(uniqueID, event));
    }

    // Starts the thread that handles the "writing to file"
    public LogAsync start() {
        // Run in another thread
        CompletableFuture.runAsync(() -> {
                    while (true) {
                        try {
                            final LogEntry entry = logEvents.take();

                            try (BufferedWriter buffWriter = new BufferedWriter(new FileWriter(entry.getUniqueId() + ".log", true))) {
                                buffWriter.write(entry.getEvent());
                            } catch (IOException e) {
                                e.printStackTrace();
                            }
                        } catch (InterruptedException e) {
                            break;
                        }
                    }
                }
        );
        return this;
    }
}

The way you could initialize the whole thing could be like this:

final LogAsync logger = new LogAsync().start();
logger.logUser(1, "Hello");
logger.logUser(1, "there");
logger.logUser(2, "Goodbye");

So, this is basically an alternative to synchronizing the whole thing. Note that this code is sample code only and there are some modifications that must be taken to make production-worthy. E.g., there must be a nice way to shutdown the writer-thread.

However, my recommendation is to not use a synchronized or asynchronous approach. Instead, use a logging framework such as SLF4J.

wassgren
  • 16,439
  • 5
  • 53
  • 71
0
public void logUser(long uniqueID, String event) throws IOException
{
    synchronized(Long.valueOf(uniqueID)) {
        BufferedWriter buffWriter = new BufferedWriter(new FileWriter(uniqueID + ".log", true));
        buffWriter.write(event);
        buffWriter.close();
    }
}
Pandiri
  • 193
  • 4
  • Hmm.. Interesting. But doesn't synchronized work on Objects themselves and not their values? – Hele Jan 02 '15 at 19:15
  • Precisely, Long.valueOf(long x) give you wrapper object of primitive long x. – Pandiri Jan 02 '15 at 19:19
  • 3
    Such a synchronization would require `Long.valueOf` to always provide the same object for the same primitive argument, which isn't guaranteed. – SE_net4 the downvoter Jan 02 '15 at 19:19
  • @E_net4 That was my question exactly. – Hele Jan 02 '15 at 19:20
  • `synchronized(Long.valueOf(uniqueID).toString().intern()) { BufferedWriter buffWriter = new BufferedWriter(new ileWriter(uniqueID + ".log", true)); buffWriter.write(event); buffWriter.close(); }` – Pandiri Jan 08 '15 at 07:13