Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bypass errors in logging for non-msft-prod environments #392

Merged
merged 7 commits into from
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
The custom logging implementation is causing segfaults in python. We'…
…re not sure exactly where, but this is the easiest and quickest way to getting a working python release.
  • Loading branch information
daxpryce committed Jul 13, 2023
commit 0f0f384f71fced7a67a0c7bd39345842d3f8cd4d
2 changes: 2 additions & 0 deletions include/abstract_data_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ template <typename data_t> class AbstractDataStore
public:
AbstractDataStore(const location_t capacity, const size_t dim);

virtual ~AbstractDataStore() = default;
daxpryce marked this conversation as resolved.
Show resolved Hide resolved

// Return number of points returned
virtual location_t load(const std::string &filename) = 0;

Expand Down
5 changes: 5 additions & 0 deletions include/logger.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,13 @@

namespace diskann
{
#ifdef ENABLE_CUSTOM_LOGGER
daxpryce marked this conversation as resolved.
Show resolved Hide resolved
DISKANN_DLLEXPORT extern std::basic_ostream<char> cout;
DISKANN_DLLEXPORT extern std::basic_ostream<char> cerr;
#else
using std::cerr;
daxpryce marked this conversation as resolved.
Show resolved Hide resolved
using std::cout;
#endif

enum class DISKANN_DLLEXPORT LogLevel
{
Expand Down
38 changes: 17 additions & 21 deletions include/logger_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace diskann
{
#ifdef ENABLE_CUSTOM_LOGGER
class ANNStreamBuf : public std::basic_streambuf<char>
{
public:
Expand All @@ -36,30 +37,25 @@ class ANNStreamBuf : public std::basic_streambuf<char>
int flush();
void logImpl(char *str, int numchars);

// Why the two buffer-sizes? If we are running normally, we are basically
// interacting with a character output system, so we short-circuit the
// output process by keeping an empty buffer and writing each character
// to stdout/stderr. But if we are running in OLS, we have to take all
// the text that is written to diskann::cout/diskann:cerr, consolidate it
// and push it out in one-shot, because the OLS infra does not give us
// character based output. Therefore, we use a larger buffer that is large
// enough to store the longest message, and continuously add characters
// to it. When the calling code outputs a std::endl or std::flush, sync()
// will be called and will output a log level, component name, and the text
// that has been collected. (sync() is also called if the buffer is full, so
// overflows/missing text are not a concern).
// This implies calling code _must_ either print std::endl or std::flush
// to ensure that the message is written immediately.
#ifdef ENABLE_CUSTOM_LOGGER
// Why the two buffer-sizes? If we are running normally, we are basically
// interacting with a character output system, so we short-circuit the
// output process by keeping an empty buffer and writing each character
// to stdout/stderr. But if we are running in OLS, we have to take all
// the text that is written to diskann::cout/diskann:cerr, consolidate it
// and push it out in one-shot, because the OLS infra does not give us
// character based output. Therefore, we use a larger buffer that is large
// enough to store the longest message, and continuously add characters
// to it. When the calling code outputs a std::endl or std::flush, sync()
// will be called and will output a log level, component name, and the text
// that has been collected. (sync() is also called if the buffer is full, so
// overflows/missing text are not a concern).
// This implies calling code _must_ either print std::endl or std::flush
// to ensure that the message is written immediately.

static const int BUFFER_SIZE = 1024;
#else
// Allocating an arbitrarily small buffer here because the overflow() and
// other function implementations push the BUFFER_SIZE chars into the
// buffer before flushing to fwrite.
static const int BUFFER_SIZE = 4;
#endif

ANNStreamBuf(const ANNStreamBuf &);
ANNStreamBuf &operator=(const ANNStreamBuf &);
};
#endif
} // namespace diskann
15 changes: 4 additions & 11 deletions src/logger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,19 @@
namespace diskann
{

#ifdef ENABLE_CUSTOM_LOGGER
DISKANN_DLLEXPORT ANNStreamBuf coutBuff(stdout);
DISKANN_DLLEXPORT ANNStreamBuf cerrBuff(stderr);

DISKANN_DLLEXPORT std::basic_ostream<char> cout(&coutBuff);
DISKANN_DLLEXPORT std::basic_ostream<char> cerr(&cerrBuff);
daxpryce marked this conversation as resolved.
Show resolved Hide resolved

#ifdef ENABLE_CUSTOM_LOGGER
std::function<void(LogLevel, const char *)> g_logger;

void SetCustomLogger(std::function<void(LogLevel, const char *)> logger)
{
g_logger = logger;
diskann::cout << "Set Custom Logger" << std::endl;
}
#endif

ANNStreamBuf::ANNStreamBuf(FILE *fp)
{
Expand All @@ -38,11 +36,7 @@ ANNStreamBuf::ANNStreamBuf(FILE *fp)
}
_fp = fp;
_logLevel = (_fp == stdout) ? LogLevel::LL_Info : LogLevel::LL_Error;
#ifdef ENABLE_CUSTOM_LOGGER
_buf = new char[BUFFER_SIZE + 1]; // See comment in the header
#else
_buf = new char[BUFFER_SIZE]; // See comment in the header
#endif

std::memset(_buf, 0, (BUFFER_SIZE) * sizeof(char));
setp(_buf, _buf + BUFFER_SIZE - 1);
Expand Down Expand Up @@ -88,17 +82,16 @@ int ANNStreamBuf::flush()
}
void ANNStreamBuf::logImpl(char *str, int num)
{
#ifdef ENABLE_CUSTOM_LOGGER
str[num] = '\0'; // Safe. See the c'tor.
// Invoke the OLS custom logging function.
if (g_logger)
{
g_logger(_logLevel, str);
}
}
#else
fwrite(str, sizeof(char), num, _fp);
fflush(_fp);
using std::cerr;
using std::cout;
#endif
}

} // namespace diskann