2
\$\begingroup\$

I wrote this code to write the log file every 5 seconds to avoid excessive disk I/O (which can cause HDD temperature to increase or SSD to wear off):

var fs = require('fs');
var logText = '';
function log(){
    fs.appendFile('c:/log.txt', logText, (err) => {
        if (err) throw err;
        logText = '';
    }); 
}
setInterval(log, 5000);

Is this the correct approach? I mean this is async right? Will setInterval interfere with the performance?

\$\endgroup\$
  • 1
    \$\begingroup\$ sometimes you have to wait for a while to get reviews from others before accepting an answer. \$\endgroup\$ – Siobhan Sep 14 '16 at 16:13
3
\$\begingroup\$

I'm assuming that you are changing the logText variable to control what's being logged. If you want to keep this same interface, the only optimization that I can see is to make log() check if logText is not empty before it writes it:

var fs = require('fs');
var logText = '';
function writelog(){
    if (logText) {
        fs.appendFile('c:/log.txt', logText, (err) => {
            if (err) throw err;
            logText = '';
        });
    }
}
setInterval(writelog, 5000);

I would also add a function that adds to the log text:

function log(text){
    logtext += ("Log prefix stuff here" + text)
}
\$\endgroup\$
  • \$\begingroup\$ exactly that. Even better because it will avoid disk I/O if there is nothing to be written. Is this async or it may interfere with the node performance? \$\endgroup\$ – Azevedo Sep 14 '16 at 14:21
  • \$\begingroup\$ If you mean the new log function, no it's not, but since it's just changing a variable, it doesn't need to be async. If you mean the writelog function, yes it is. It shouldn't impact performance, but if you are worried about it, try profiling your program with and without the log to see if it noticeable impacting performance. \$\endgroup\$ – BookOwl Sep 14 '16 at 14:28
  • \$\begingroup\$ Thanks, very helpful. I'll just add + '\n'to logtext += and the timestamp. \$\endgroup\$ – Azevedo Sep 14 '16 at 14:33
  • \$\begingroup\$ @Azevedo This isn't an async approach in the way you probably want, since it can cause a race condition if appendFile takes longer than 5 seconds. You want to create a queue to avoid that. Put logQueue = [] outside of the function, before appendFile logQueue.push(text), and in appendFile something like logQueue.unshift() as the logText \$\endgroup\$ – antimatter Sep 14 '16 at 22:16
3
\$\begingroup\$

Let the operating system handle write caching for you.

\$\endgroup\$
2
\$\begingroup\$

You are writing roughly the same amount of data to disk whether you wait to do it in batches or do it right away. So I don't really know how valid of a concern you really have here. It seems like you are solving a problem that doesn't really need to be solved.

Also what happens when something goes wrong 4 seconds into a 5 second log cycle? Do you lose all the log data for that 4 seconds (as well as your ability to trace the problem)? Do you lose timestamp granularity for the actual items being logged?

This seems like a step backwards in making your application easy to maintain.

If you are this worried about writing log files to disk, then perhaps consider a central log store that you can log to over UDP or similar.

You also must consider that this approach would require more memory utilization within your application to store the continually growing (until logged) log string. Depending on the amount of data being logged, this could potentially be a much more important consideration. Having to add application nodes because your code is not efficient with memory usage is likely MUCH more expensive than incremental cost of replacing hard drives at some theoretically more frequent rate would be.

Bottom line - memory is typically more expensive than storage.

\$\endgroup\$
  • \$\begingroup\$ The main concern here is the disk I/O frequency, not the amount of data written. The purpose of this function is log activities, notices. The 5s gap is not a problem. If the application crashes and won't log the last 3 seconds it is not a problem. The log is for the administrator, not for the developer. \$\endgroup\$ – Azevedo Sep 14 '16 at 16:50
  • \$\begingroup\$ @Azevedo And why is i/o frequency a concern? And is it better to have more consistent logging load vs. spiky loads? How is this service scaling? Is it worth introducing this change to "improve" hard drive performance/lifecycle for a handful of (relatively) low-priced commodity drives? Honestly, if you are that high level of use and number of drives impacted to where this actually has any business value, my guess is that you are well beyond the point of needing to have moved to a centralized logging solution. \$\endgroup\$ – Mike Brant Sep 14 '16 at 18:12
  • \$\begingroup\$ @Azevedo And apologies if it seems like I am doubting your need for this, but in many years of doing software development, including a number of years dealing with EXTREMELY high volume systems, I have never once come across a case where someone is wanting to optimize hard drive operational characteristics by batching log operations. Hard drives are cheap. Developers are not. I would optimize towards a solution that makes this more maintainable for developers, even if it means you have to replace hard drives 1% more frequently than you otherwise would have. \$\endgroup\$ – Mike Brant Sep 14 '16 at 18:15
  • \$\begingroup\$ @Azvedo Updated original answer to comment on memory utilization of this approach. \$\endgroup\$ – Mike Brant Sep 14 '16 at 19:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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