From 16f8b8e43f318bd95471353bd02faa13071a7126 Mon Sep 17 00:00:00 2001 From: Andrea Cavalli Date: Thu, 24 Oct 2024 02:07:12 +0200 Subject: [PATCH] Fix error printing, stable column names --- .../rockserver/core/impl/EmbeddedDB.java | 90 +++++++++++-------- .../rockserver/core/server/GrpcServer.java | 7 +- 2 files changed, 55 insertions(+), 42 deletions(-) diff --git a/src/main/java/it/cavallium/rockserver/core/impl/EmbeddedDB.java b/src/main/java/it/cavallium/rockserver/core/impl/EmbeddedDB.java index 2d736ac..de55477 100644 --- a/src/main/java/it/cavallium/rockserver/core/impl/EmbeddedDB.java +++ b/src/main/java/it/cavallium/rockserver/core/impl/EmbeddedDB.java @@ -176,19 +176,29 @@ public class EmbeddedDB implements RocksDBSyncAPI, Closeable { * Do not try to register a column that may already be registered */ private long registerColumn(@NotNull ColumnInstance column) { - try { - var columnName = new String(column.cfh().getName(), StandardCharsets.UTF_8); - long id = FastRandomUtils.allocateNewValue(this.columns, column, 1, Long.MAX_VALUE); - Long previous = this.columnNamesIndex.putIfAbsent(columnName, id); - if (previous != null) { - //noinspection resource - this.columns.remove(id); - throw new UnsupportedOperationException("Column already registered!"); + synchronized (columnEditLock) { + try { + var columnName = new String(column.cfh().getName(), StandardCharsets.UTF_8); + long hashCode = columnName.hashCode(); + long id; + if (this.columnNamesIndex.get(columnName) == hashCode) { + id = hashCode; + } else if (this.columns.get(hashCode) == null) { + id = hashCode; + } else { + id = FastRandomUtils.allocateNewValue(this.columns, column, 1, Long.MAX_VALUE); + } + Long previous = this.columnNamesIndex.putIfAbsent(columnName, id); + if (previous != null) { + //noinspection resource + this.columns.remove(id); + throw new UnsupportedOperationException("Column already registered!"); + } + logger.info("Registered column: " + column); + return id; + } catch (org.rocksdb.RocksDBException e) { + throw new RuntimeException(e); } - logger.info("Registered column: " + column); - return id; - } catch (org.rocksdb.RocksDBException e) { - throw new RuntimeException(e); } } @@ -197,35 +207,37 @@ public class EmbeddedDB implements RocksDBSyncAPI, Closeable { * Do not try to unregister a column that may already be unregistered, or that may not be registered */ private ColumnInstance unregisterColumn(long id) { - var col = this.columns.remove(id); - Objects.requireNonNull(col, () -> "Column does not exist: " + id); - String name; - try { - name = new String(col.cfh().getName(), StandardCharsets.UTF_8); - } catch (org.rocksdb.RocksDBException e) { - throw new RuntimeException(e); - } - // Unregister the column name from the index avoiding race conditions - int retries = 0; - while (this.columnNamesIndex.remove(name) == null && retries++ < 5_000) { - Thread.yield(); - } - if (retries >= 5000) { - throw new IllegalStateException("Can't find column in column names index: " + name); - } + synchronized (columnEditLock) { + var col = this.columns.remove(id); + Objects.requireNonNull(col, () -> "Column does not exist: " + id); + String name; + try { + name = new String(col.cfh().getName(), StandardCharsets.UTF_8); + } catch (org.rocksdb.RocksDBException e) { + throw new RuntimeException(e); + } + // Unregister the column name from the index avoiding race conditions + int retries = 0; + while (this.columnNamesIndex.remove(name) == null && retries++ < 5_000) { + Thread.yield(); + } + if (retries >= 5000) { + throw new IllegalStateException("Can't find column in column names index: " + name); + } - ColumnFamilyOptions columnConfig; - while ((columnConfig = this.columnsConifg.remove(name)) == null && retries++ < 5_000) { - Thread.yield(); - } - if (columnConfig != null) { - columnConfig.close(); - } - if (retries >= 5000) { - throw new IllegalStateException("Can't find column in column names index: " + name); - } + ColumnFamilyOptions columnConfig; + while ((columnConfig = this.columnsConifg.remove(name)) == null && retries++ < 5_000) { + Thread.yield(); + } + if (columnConfig != null) { + columnConfig.close(); + } + if (retries >= 5000) { + throw new IllegalStateException("Can't find column in column names index: " + name); + } - return col; + return col; + } } @Override diff --git a/src/main/java/it/cavallium/rockserver/core/server/GrpcServer.java b/src/main/java/it/cavallium/rockserver/core/server/GrpcServer.java index eaf7513..1b48d03 100644 --- a/src/main/java/it/cavallium/rockserver/core/server/GrpcServer.java +++ b/src/main/java/it/cavallium/rockserver/core/server/GrpcServer.java @@ -542,8 +542,9 @@ public class GrpcServer extends Server { private Function, Flux> onErrorMapFluxWithRequestInfo(String requestName, Message request) { return flux -> flux.onErrorResume(throwable -> { var ex = handleError(throwable).asException(); - if (ex.getStatus().getCode() == Code.INTERNAL) { - LOG.error("Unexpected internal error during request: {}", request, ex); + if (ex.getStatus().getCode() == Code.INTERNAL && !(throwable instanceof RocksDBException)) { + LOG.error("Unexpected internal error during request \"{}\": {}", requestName, request.toString(), ex); + return Mono.error(RocksDBException.of(RocksDBErrorType.INTERNAL_ERROR, ex.getCause())); } return Mono.error(ex); }); @@ -552,7 +553,7 @@ public class GrpcServer extends Server { private Function, Mono> onErrorMapMonoWithRequestInfo(String requestName, Message request) { return flux -> flux.onErrorResume(throwable -> { var ex = handleError(throwable).asException(); - if (ex.getStatus().getCode() == Code.INTERNAL) { + if (ex.getStatus().getCode() == Code.INTERNAL && !(throwable instanceof RocksDBException)) { LOG.error("Unexpected internal error during request \"{}\": {}", requestName, request.toString(), ex); return Mono.error(RocksDBException.of(RocksDBErrorType.INTERNAL_ERROR, ex.getCause())); }