You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/07/13 18:24:57 UTC
[GitHub] [ignite-3] rpuch opened a new pull request, #939: IGNITE-17334 Basic volatile RAFT log storage
rpuch opened a new pull request, #939:
URL: https://github.com/apache/ignite-3/pull/939
https://issues.apache.org/jira/browse/IGNITE-17334
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
rpuch commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r940294899
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/impl/EntryCountBudget.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import java.util.List;
+import org.apache.ignite.raft.jraft.entity.LogEntry;
+
+/**
+ * {@link LogStorageBudget} that makes sure that no more entries than the provided limit is stored.
+ */
+public class EntryCountBudget implements LogStorageBudget {
Review Comment:
All the methods of `LogStorageBudget` are called by `VoolatileLogStorage` under locks, so it seems that the budget implementation should not worry about thread safety.
WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
rpuch commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r939890717
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/impl/LogStorageOverflowException.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import org.apache.ignite.lang.IgniteException;
+
+/**
+ * Thrown by {@link org.apache.ignite.raft.jraft.storage.LogStorage} methods to indicate that some entries (a suffix of the
+ * collection of entries that was tried to be saved) were not saved due to an overflow.
+ */
+public class LogStorageOverflowException extends IgniteException {
+ private final int overflowedEntries;
+
+ /**
+ * Constructs new instance.
+ *
+ * @param overflowedEntries Length of the suffix that was not saved.
+ */
+ public LogStorageOverflowException(int overflowedEntries) {
+ super(overflowedEntries + " entries were not saved due to log storage overflow");
+
+ this.overflowedEntries = overflowedEntries;
+ }
+
+ /**
+ * Returns number of entries that were not saved in the current attempt to append entries.
+ *
+ * @return Number of entries that were not saved in the current attempt to append entries.
Review Comment:
Why should we omit `@return` tag, is there a style rule about this? I thought we had to always use `@return` tag for non-void methods.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] alievmirza commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
alievmirza commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r946888230
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/impl/VolatileLogStorage.java:
##########
@@ -0,0 +1,299 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import java.util.List;
+import java.util.NavigableMap;
+import java.util.SortedMap;
+import java.util.concurrent.ConcurrentSkipListMap;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.raft.jraft.entity.EnumOutter;
+import org.apache.ignite.raft.jraft.entity.LogEntry;
+import org.apache.ignite.raft.jraft.entity.LogId;
+import org.apache.ignite.raft.jraft.entity.codec.LogEntryDecoder;
+import org.apache.ignite.raft.jraft.entity.codec.LogEntryEncoder;
+import org.apache.ignite.raft.jraft.option.LogStorageOptions;
+import org.apache.ignite.raft.jraft.storage.LogStorage;
+import org.apache.ignite.raft.jraft.storage.VolatileStorage;
+import org.apache.ignite.raft.jraft.util.Describer;
+import org.apache.ignite.raft.jraft.util.Requires;
+
+/**
+ * Stores RAFT log in memory.
+ */
+public class VolatileLogStorage implements LogStorage, Describer, VolatileStorage {
+ private static final IgniteLogger LOG = Loggers.forClass(VolatileLogStorage.class);
+
+ private final LogStorageBudget budget;
+
+ private final ReadWriteLock readWriteLock = new ReentrantReadWriteLock();
+ private final Lock readLock = this.readWriteLock.readLock();
+ private final Lock writeLock = this.readWriteLock.writeLock();
+
+ private final NavigableMap<Long, LogEntry> log = new ConcurrentSkipListMap<>();
+
+ private LogEntryEncoder logEntryEncoder;
+ private LogEntryDecoder logEntryDecoder;
+
+ private volatile long firstLogIndex = 1;
+ private volatile long lastLogIndex = 0;
+
+ private volatile boolean initialized = false;
+
+ public VolatileLogStorage(LogStorageBudget budget) {
+ super();
+
+ this.budget = budget;
+ }
+
+ @Override
+ public boolean init(final LogStorageOptions opts) {
+ Requires.requireNonNull(opts.getConfigurationManager(), "Null conf manager");
+ Requires.requireNonNull(opts.getLogEntryCodecFactory(), "Null log entry codec factory");
+
+ this.writeLock.lock();
+
+ try {
+ if (initialized) {
+ LOG.warn("VolatileLogStorage init() was already called.");
+ return true;
+ }
+ this.initialized = true;
+ this.logEntryDecoder = opts.getLogEntryCodecFactory().decoder();
+ this.logEntryEncoder = opts.getLogEntryCodecFactory().encoder();
+ Requires.requireNonNull(this.logEntryDecoder, "Null log entry decoder");
+ Requires.requireNonNull(this.logEntryEncoder, "Null log entry encoder");
+
+ return true;
+ } finally {
+ this.writeLock.unlock();
+ }
+ }
+
+ @Override
+ public void shutdown() {
+ this.writeLock.lock();
+
+ try {
+ this.initialized = false;
+ this.log.clear();
+ } finally {
+ this.writeLock.unlock();
+ }
+ }
+
+ @Override
+ public long getFirstLogIndex() {
+ this.readLock.lock();
+
+ try {
+ return this.firstLogIndex;
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public long getLastLogIndex() {
+ this.readLock.lock();
+
+ try {
+ return this.lastLogIndex;
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public LogEntry getEntry(final long index) {
+ this.readLock.lock();
+
+ try {
+ if (index < getFirstLogIndex()) {
+ return null;
+ }
+
+ return log.get(index);
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public long getTerm(final long index) {
+ final LogEntry entry = getEntry(index);
+ if (entry != null) {
+ return entry.getId().getTerm();
+ }
+ return 0;
+ }
+
+ @Override
+ public boolean appendEntry(final LogEntry entry) {
+ this.readLock.lock();
+
+ try {
+ if (!initialized) {
+ LOG.warn("DB not initialized or destroyed.");
+ return false;
+ }
+
+ this.log.put(entry.getId().getIndex(), entry);
+
+ lastLogIndex = log.lastKey();
+ firstLogIndex = log.firstKey();
+
+ budget.onAppended(entry);
+
+ return true;
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public int appendEntries(final List<LogEntry> entries) {
+ if (entries == null || entries.isEmpty()) {
+ return 0;
+ }
+
+ final int entriesCount = entries.size();
+
+ this.readLock.lock();
+
+ try {
+ if (!initialized) {
+ LOG.warn("DB not initialized or destroyed.");
+ return 0;
+ }
+
+ int appended = 0;
+ for (LogEntry logEntry : entries) {
+
+ log.put(logEntry.getId().getIndex(), logEntry);
+
+ appended++;
+ }
+
+ lastLogIndex = log.lastKey();
+ firstLogIndex = log.firstKey();
+
+ budget.onAppended(entries.subList(0, appended));
+
+ if (appended < entriesCount) {
Review Comment:
Seems like this code is redundant
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/LogManager.java:
##########
@@ -220,5 +220,4 @@ interface NewLogCallback {
* @return status
*/
Status checkConsistency();
-
}
Review Comment:
EOL
##########
modules/raft/src/test/java/org/apache/ignite/raft/jraft/storage/impl/UnlimitedBudgetTest.java:
##########
@@ -0,0 +1,32 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.apache.ignite.raft.jraft.entity.LogEntry;
+import org.junit.jupiter.api.Test;
+
+class UnlimitedBudgetTest {
+ private final UnlimitedBudget budget = new UnlimitedBudget();
+
+ @Test
+ void allowsAppend() {
+ assertTrue(budget.hasRoomFor(new LogEntry()));
+ }
+}
Review Comment:
EOL
##########
modules/raft/src/test/java/org/apache/ignite/raft/jraft/storage/impl/EntryCountBudgetTest.java:
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.List;
+import org.apache.ignite.raft.jraft.entity.LogEntry;
+import org.apache.ignite.raft.jraft.entity.LogId;
+import org.junit.jupiter.api.Test;
+
+class EntryCountBudgetTest {
+ @Test
+ void allowsAppendWhenThereIsRoom() {
+ EntryCountBudget budget = new EntryCountBudget(1);
+
+ assertTrue(budget.hasRoomFor(entry(1)));
+ }
+
+ private LogEntry entry(long index) {
+ LogEntry entry = new LogEntry();
+ entry.setId(new LogId(index, 1));
+ return entry;
+ }
+
+ @Test
+ void deniesAppendWhenThereIsNoRoom() {
+ EntryCountBudget budget = new EntryCountBudget(0);
+
+ assertFalse(budget.hasRoomFor(entry(1)));
+ }
+
+ @Test
+ void onAppendedSingleDecreasesRoom() {
+ EntryCountBudget budget = new EntryCountBudget(1);
+
+ budget.onAppended(entry(1));
+
+ assertFalse(budget.hasRoomFor(entry(2)));
+ }
+
+ @Test
+ void onAppendedMultipleDecreasesRoom() {
+ EntryCountBudget budget = new EntryCountBudget(2);
+
+ budget.onAppended(List.of(entry(1), entry(2)));
+
+ assertFalse(budget.hasRoomFor(entry(3)));
+ }
+
+ @Test
+ void onTruncatedPrefixForStoredPrefixIncreasesRoom() {
+ EntryCountBudget budget = new EntryCountBudget(2);
+
+ budget.onAppended(List.of(entry(1), entry(2)));
+
+ budget.onTruncatedPrefix(2);
+
+ assertTrue(budget.hasRoomFor(entry(3)));
+ }
+
+ @Test
+ void onTruncatedPrefixForNonStoredPrefixDoesNotIncreaseRoom() {
+ EntryCountBudget budget = new EntryCountBudget(2);
+
+ budget.onAppended(List.of(entry(1), entry(2)));
+
+ budget.onTruncatedPrefix(2);
+
+ budget.onAppended(List.of(entry(3)));
+
+ budget.onTruncatedPrefix(2);
+
+ assertFalse(budget.hasRoomFor(entry(4)));
+ }
+
+ @Test
+ void onTruncatedSuffixForStoredPrefixIncreasesRoom() {
+ EntryCountBudget budget = new EntryCountBudget(2);
+
+ budget.onAppended(List.of(entry(1), entry(2)));
+
+ budget.onTruncatedSuffix(1);
+
+ assertTrue(budget.hasRoomFor(entry(2)));
+ }
+
+ @Test
+ void onTruncatedSuffixForNonStoredPrefixDoesNotIncreaseRoom() {
+ EntryCountBudget budget = new EntryCountBudget(2);
+
+ budget.onAppended(List.of(entry(1), entry(2)));
+
+ budget.onTruncatedSuffix(2);
+
+ assertFalse(budget.hasRoomFor(entry(3)));
+ }
+}
Review Comment:
EOL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
rpuch commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r939884383
##########
.gitignore:
##########
@@ -4,3 +4,8 @@ target
work
.DS_Store
.flattened-pom.xml
+
+.classpath
Review Comment:
These are Eclipse-related files/dirs (like *.iml for Idea). Eclipse creates them when you import a project to Eclipse.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
rpuch commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r947078573
##########
modules/api/src/main/java/org/apache/ignite/configuration/schemas/table/EntryCountBudgetConfigurationSchema.java:
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.configuration.schemas.table;
+
+import org.apache.ignite.configuration.annotation.PolymorphicConfigInstance;
+import org.apache.ignite.configuration.annotation.Value;
+
+/**
+ * Configuration for 'entry-count' log storage budget.
Review Comment:
The corresponding interface (and its implementations) are not available in this module, so it's impossible to add a link to them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
rpuch commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r947071944
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/impl/EntryCountBudget.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import java.util.List;
+import org.apache.ignite.raft.jraft.entity.LogEntry;
+
+/**
+ * {@link LogStorageBudget} that makes sure that no more entries than the provided limit is stored.
+ *
+ * <p>This is not thread safe as {@link LogStorageBudget} implementations do not need to be thread safe (because all budget methods
+ * are called under locks).
+ */
+public class EntryCountBudget implements LogStorageBudget {
+
+ private static final long NO_INDEX = -1;
+
+ private final long entriesCountLimit;
+
+ private long firstIndex = NO_INDEX;
+ private long lastIndex = NO_INDEX;
+
+ public EntryCountBudget(long entriesCountLimit) {
+ this.entriesCountLimit = entriesCountLimit;
+ }
+
+ @Override
+ public boolean hasRoomFor(LogEntry entry) {
+ return storedEntries() < entriesCountLimit;
+ }
+
+ private long storedEntries() {
+ if (firstIndex == NO_INDEX && lastIndex == NO_INDEX) {
+ return 0;
+ } else if (firstIndex != NO_INDEX && lastIndex != NO_INDEX) {
+ return lastIndex - firstIndex + 1;
+ } else {
+ throw new IllegalStateException("Only one of firstIndex and lastIndex is initialized: " + firstIndex + " and " + lastIndex);
+ }
+ }
+
+ @Override
+ public void onAppended(LogEntry entry) {
Review Comment:
A budget accounting for entry sizes will actually need `LogEntry`. Such a budget will be added later, in IGNITE-17337
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] ibessonov merged pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
ibessonov merged PR #939:
URL: https://github.com/apache/ignite-3/pull/939
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
rpuch commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r947080181
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/impl/VolatileLogStorage.java:
##########
@@ -0,0 +1,289 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import java.util.List;
+import java.util.NavigableMap;
+import java.util.SortedMap;
+import java.util.concurrent.ConcurrentSkipListMap;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.raft.jraft.entity.EnumOutter;
+import org.apache.ignite.raft.jraft.entity.LogEntry;
+import org.apache.ignite.raft.jraft.entity.LogId;
+import org.apache.ignite.raft.jraft.entity.codec.LogEntryDecoder;
+import org.apache.ignite.raft.jraft.entity.codec.LogEntryEncoder;
+import org.apache.ignite.raft.jraft.option.LogStorageOptions;
+import org.apache.ignite.raft.jraft.storage.LogStorage;
+import org.apache.ignite.raft.jraft.storage.VolatileStorage;
+import org.apache.ignite.raft.jraft.util.Describer;
+import org.apache.ignite.raft.jraft.util.Requires;
+
+/**
+ * Stores RAFT log in memory.
+ */
+public class VolatileLogStorage implements LogStorage, Describer, VolatileStorage {
+ private static final IgniteLogger LOG = Loggers.forClass(VolatileLogStorage.class);
+
+ private final LogStorageBudget budget;
+
+ private final ReadWriteLock readWriteLock = new ReentrantReadWriteLock();
+ private final Lock readLock = this.readWriteLock.readLock();
+ private final Lock writeLock = this.readWriteLock.writeLock();
+
+ private final NavigableMap<Long, LogEntry> log = new ConcurrentSkipListMap<>();
+
+ private LogEntryEncoder logEntryEncoder;
+ private LogEntryDecoder logEntryDecoder;
+
+ private volatile long firstLogIndex = 1;
+ private volatile long lastLogIndex = 0;
+
+ private volatile boolean initialized = false;
+
+ public VolatileLogStorage(LogStorageBudget budget) {
+ super();
+
+ this.budget = budget;
+ }
+
+ @Override
+ public boolean init(final LogStorageOptions opts) {
+ Requires.requireNonNull(opts.getConfigurationManager(), "Null conf manager");
+ Requires.requireNonNull(opts.getLogEntryCodecFactory(), "Null log entry codec factory");
+
+ this.writeLock.lock();
+
+ try {
+ if (initialized) {
+ LOG.warn("VolatileLogStorage init() was already called.");
+ return true;
+ }
+ this.initialized = true;
+ this.logEntryDecoder = opts.getLogEntryCodecFactory().decoder();
+ this.logEntryEncoder = opts.getLogEntryCodecFactory().encoder();
+ Requires.requireNonNull(this.logEntryDecoder, "Null log entry decoder");
+ Requires.requireNonNull(this.logEntryEncoder, "Null log entry encoder");
+
+ return true;
+ } finally {
+ this.writeLock.unlock();
+ }
+ }
+
+ @Override
+ public void shutdown() {
+ this.writeLock.lock();
+
+ try {
+ this.initialized = false;
+ this.log.clear();
+ } finally {
+ this.writeLock.unlock();
+ }
+ }
+
+ @Override
+ public long getFirstLogIndex() {
+ this.readLock.lock();
+
+ try {
+ return this.firstLogIndex;
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public long getLastLogIndex() {
+ this.readLock.lock();
+
+ try {
+ return this.lastLogIndex;
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public LogEntry getEntry(final long index) {
+ this.readLock.lock();
+
+ try {
+ if (index < getFirstLogIndex()) {
+ return null;
+ }
+
+ return log.get(index);
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public long getTerm(final long index) {
+ final LogEntry entry = getEntry(index);
+ if (entry != null) {
+ return entry.getId().getTerm();
+ }
+ return 0;
+ }
+
+ @Override
+ public boolean appendEntry(final LogEntry entry) {
+ this.readLock.lock();
+
+ try {
+ if (!initialized) {
+ LOG.warn("DB not initialized or destroyed.");
+ return false;
+ }
+
+ this.log.put(entry.getId().getIndex(), entry);
+
+ lastLogIndex = log.lastKey();
+ firstLogIndex = log.firstKey();
+
+ budget.onAppended(entry);
+
+ return true;
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public int appendEntries(final List<LogEntry> entries) {
+ if (entries == null || entries.isEmpty()) {
+ return 0;
+ }
+
+ final int entriesCount = entries.size();
+
+ this.readLock.lock();
+
+ try {
+ if (!initialized) {
+ LOG.warn("DB not initialized or destroyed.");
+ return 0;
+ }
+
+ for (LogEntry logEntry : entries) {
+ log.put(logEntry.getId().getIndex(), logEntry);
+ }
+
+ lastLogIndex = log.lastKey();
+ firstLogIndex = log.firstKey();
+
+ budget.onAppended(entries);
Review Comment:
Added a TODO
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r940919316
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/LogStorageBudgetsModule.java:
##########
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.raft.jraft.core;
+
+import java.util.Map;
+import java.util.function.Function;
+import org.apache.ignite.configuration.schemas.table.LogStorageBudgetView;
+import org.apache.ignite.raft.jraft.storage.impl.LogStorageBudget;
+
+/**
+ * Used to add {@link LogStorageBudget} factories using {@link java.util.ServiceLoader}.
+ */
+public interface LogStorageBudgetsModule {
+ /**
+ * Returns mapping from budget names to budget factories for the budgets supported by this module.
+ *
+ * @return Mapping from budget names to budget factories for the budgets supported by this module.
Review Comment:
```suggestion
```
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/LogStorageBudgetsModule.java:
##########
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.raft.jraft.core;
+
+import java.util.Map;
+import java.util.function.Function;
+import org.apache.ignite.configuration.schemas.table.LogStorageBudgetView;
+import org.apache.ignite.raft.jraft.storage.impl.LogStorageBudget;
+
+/**
+ * Used to add {@link LogStorageBudget} factories using {@link java.util.ServiceLoader}.
+ */
+public interface LogStorageBudgetsModule {
+ /**
+ * Returns mapping from budget names to budget factories for the budgets supported by this module.
+ *
+ * @return Mapping from budget names to budget factories for the budgets supported by this module.
+ */
+ Map<String, Function<? super LogStorageBudgetView, LogStorageBudget>> budgetFactories();
Review Comment:
Why not create a separate factory interface? it is also not necessary to take the form, you can just config.
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/impl/UnlimitedBudget.java:
##########
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import org.apache.ignite.raft.jraft.entity.LogEntry;
+
+/**
+ * {@link LogStorageBudget} that always allows everything.
+ */
+public class UnlimitedBudget implements LogStorageBudget {
+
Review Comment:
```suggestion
```
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/impl/LogStorageBudget.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import java.util.List;
+import org.apache.ignite.raft.jraft.entity.LogEntry;
+
+/**
+ * Knows how to determine whether there is still room for more log entries.
+ */
+public interface LogStorageBudget {
+ /**
+ * Returns {@code true} if there is room for the given entry, {@code false} otherwise.
+ *
+ * @param entry Entry that is tried to be appended.
+ * @return {@code true} if there is room for the given entry, {@code false} otherwise.
Review Comment:
```suggestion
```
##########
modules/api/src/main/java/org/apache/ignite/configuration/schemas/table/TableConfigurationSchema.java:
##########
@@ -61,4 +61,8 @@ public class TableConfigurationSchema {
/** Indices configuration. */
@NamedConfigValue
public TableIndexConfigurationSchema indices;
+
+ /** Configuration for Raft groups corresponding to table partitions. */
+ @ConfigValue
+ public RaftConfigurationSchema raft;
Review Comment:
How is this related to the table and how can the user configure it?
I think it should be taken out in a configuration where it is spoken about the table in-memory.
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/impl/EntryCountBudget.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import java.util.List;
+import org.apache.ignite.raft.jraft.entity.LogEntry;
+
+/**
+ * {@link LogStorageBudget} that makes sure that no more entries than the provided limit is stored.
+ */
+public class EntryCountBudget implements LogStorageBudget {
Review Comment:
Write in the documentation about it.
##########
modules/api/src/main/java/org/apache/ignite/configuration/schemas/table/EntryCountBudgetConfigurationSchema.java:
##########
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.configuration.schemas.table;
+
+import org.apache.ignite.configuration.annotation.PolymorphicConfigInstance;
+import org.apache.ignite.configuration.annotation.Value;
+
+/**
+ * Configuration for 'unlimited' log storage budget.
+ */
+@PolymorphicConfigInstance(EntryCountBudgetConfigurationSchema.NAME)
+public class EntryCountBudgetConfigurationSchema extends LogStorageBudgetConfigurationSchema {
+ /** The budget name. */
+ public static final String NAME = "entry-count";
+
+ @Value
+ public long entriesCountLimit;
Review Comment:
Please add javadoc
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r924297270
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/server/RaftGroupOptions.java:
##########
@@ -17,13 +17,18 @@
package org.apache.ignite.internal.raft.server;
+import org.jetbrains.annotations.Nullable;
+
/**
* Options specific to a Raft group that is being started.
*/
public class RaftGroupOptions {
/** Whether volatile stores should be used for the corresponding Raft Group. Classic Raft uses persistent ones. */
private final boolean volatileStores;
+ @Nullable
+ private final String volatileLogStoreBudgetClassName;
Review Comment:
```suggestion
private final @Nullable String volatileLogStoreBudgetClassName;
```
##########
modules/api/src/main/java/org/apache/ignite/configuration/schemas/table/RaftConfigurationSchema.java:
##########
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.configuration.schemas.table;
+
+import org.apache.ignite.configuration.annotation.Config;
+import org.apache.ignite.configuration.annotation.Value;
+import org.apache.ignite.configuration.validation.Immutable;
+
+/**
+ * Configuration for Raft group corresponding to tables.
+ */
+@Config
+public class RaftConfigurationSchema {
+ /** Class of log storage budget. */
+ @Value(hasDefault = true)
+ @Immutable
+ public String logStorageBudgetClass = "org.apache.ignite.raft.jraft.storage.impl.UnlimitedBudget";
Review Comment:
Is it correct for the user to set this through the configuration? Maybe through loading services or something like that?
##########
modules/api/src/main/java/org/apache/ignite/configuration/schemas/table/RaftConfigurationSchema.java:
##########
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.configuration.schemas.table;
+
+import org.apache.ignite.configuration.annotation.Config;
+import org.apache.ignite.configuration.annotation.Value;
+import org.apache.ignite.configuration.validation.Immutable;
+
+/**
+ * Configuration for Raft group corresponding to tables.
+ */
+@Config
+public class RaftConfigurationSchema {
+ /** Class of log storage budget. */
+ @Value(hasDefault = true)
+ @Immutable
Review Comment:
Why should it be immutable? after node restart can we change it?
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/impl/LogStorageOverflowException.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import org.apache.ignite.lang.IgniteException;
+
+/**
+ * Thrown by {@link org.apache.ignite.raft.jraft.storage.LogStorage} methods to indicate that some entries (a suffix of the
+ * collection of entries that was tried to be saved) were not saved due to an overflow.
+ */
+public class LogStorageOverflowException extends IgniteException {
+ private final int overflowedEntries;
+
+ /**
+ * Constructs new instance.
+ *
+ * @param overflowedEntries Length of the suffix that was not saved.
+ */
+ public LogStorageOverflowException(int overflowedEntries) {
+ super(overflowedEntries + " entries were not saved due to log storage overflow");
+
+ this.overflowedEntries = overflowedEntries;
+ }
+
+ /**
+ * Returns number of entries that were not saved in the current attempt to append entries.
+ *
+ * @return Number of entries that were not saved in the current attempt to append entries.
Review Comment:
```suggestion
* Returns number of entries that were not saved in the current attempt to append entries.
```
##########
modules/api/src/main/java/org/apache/ignite/configuration/schemas/table/TableConfigurationSchema.java:
##########
@@ -61,4 +61,8 @@ public class TableConfigurationSchema {
/** Indices configuration. */
@NamedConfigValue
public TableIndexConfigurationSchema indices;
+
+ /** Configuration for Raft groups corresponding to table partitions. */
+ @ConfigValue
+ public RaftConfigurationSchema raft;
Review Comment:
I don't know if this is correct or not, but I have a feeling that it is not. @sanpwc WDYT?
##########
.gitignore:
##########
@@ -4,3 +4,8 @@ target
work
.DS_Store
.flattened-pom.xml
+
+.classpath
Review Comment:
At what point are these files/directories created?
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/server/RaftGroupOptions.java:
##########
@@ -75,4 +81,15 @@ private RaftGroupOptions(boolean volatileStores) {
public boolean volatileStores() {
return volatileStores;
}
+
+ /**
+ * Returns name of a class implementing {@link org.apache.ignite.raft.jraft.storage.impl.LogStorageBudget} that
+ * will be used by volatile Raft log storage (if it's used).
+ *
+ * @return Name of a budget class.
+ */
+ @Nullable
+ public String volatileLogStoreBudgetClassName() {
Review Comment:
```suggestion
public @Nullable String volatileLogStoreBudgetClassName() {
```
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/impl/EntryCountBudget.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import java.util.List;
+import org.apache.ignite.raft.jraft.entity.LogEntry;
+
+/**
+ * {@link LogStorageBudget} that makes sure that no more entries than the provided limit is stored.
+ */
+public class EntryCountBudget implements LogStorageBudget {
Review Comment:
Should it work in one thread?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
rpuch commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r947070511
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/impl/EntryCountBudget.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import java.util.List;
+import org.apache.ignite.raft.jraft.entity.LogEntry;
+
+/**
+ * {@link LogStorageBudget} that makes sure that no more entries than the provided limit is stored.
+ *
+ * <p>This is not thread safe as {@link LogStorageBudget} implementations do not need to be thread safe (because all budget methods
+ * are called under locks).
+ */
+public class EntryCountBudget implements LogStorageBudget {
+
+ private static final long NO_INDEX = -1;
+
+ private final long entriesCountLimit;
+
+ private long firstIndex = NO_INDEX;
+ private long lastIndex = NO_INDEX;
+
+ public EntryCountBudget(long entriesCountLimit) {
+ this.entriesCountLimit = entriesCountLimit;
+ }
+
+ @Override
+ public boolean hasRoomFor(LogEntry entry) {
Review Comment:
Later, we'll have another implementation that will account for total entries size, it will need `entry` in `hasRoomFor()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r940913404
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/impl/LogStorageOverflowException.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import org.apache.ignite.lang.IgniteException;
+
+/**
+ * Thrown by {@link org.apache.ignite.raft.jraft.storage.LogStorage} methods to indicate that some entries (a suffix of the
+ * collection of entries that was tried to be saved) were not saved due to an overflow.
+ */
+public class LogStorageOverflowException extends IgniteException {
+ private final int overflowedEntries;
+
+ /**
+ * Constructs new instance.
+ *
+ * @param overflowedEntries Length of the suffix that was not saved.
+ */
+ public LogStorageOverflowException(int overflowedEntries) {
+ super(overflowedEntries + " entries were not saved due to log storage overflow");
+
+ this.overflowedEntries = overflowedEntries;
+ }
+
+ /**
+ * Returns number of entries that were not saved in the current attempt to append entries.
+ *
+ * @return Number of entries that were not saved in the current attempt to append entries.
Review Comment:
In 2.0 it was definitely required, in 3.0 it is not necessary so this is a suggestion and not a remark.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
rpuch commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r944761133
##########
modules/api/src/main/java/org/apache/ignite/configuration/schemas/table/TableConfigurationSchema.java:
##########
@@ -61,4 +61,8 @@ public class TableConfigurationSchema {
/** Indices configuration. */
@NamedConfigValue
public TableIndexConfigurationSchema indices;
+
+ /** Configuration for Raft groups corresponding to table partitions. */
+ @ConfigValue
+ public RaftConfigurationSchema raft;
Review Comment:
On a phone call, it was decided to leave it more or less as it is for now. I changed a few names and added a TODO, as it was agreed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
rpuch commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r947083961
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/impl/VolatileLogStorageFactory.java:
##########
@@ -34,7 +83,21 @@ public void start() {
/** {@inheritDoc} */
@Override
public LogStorage createLogStorage(String uri, RaftOptions raftOptions) {
- return new LocalLogStorage(raftOptions);
+ return new VolatileLogStorage(createLogStorageBudget());
+ }
+
+ private LogStorageBudget createLogStorageBudget() {
+ return newBudget(logStorageBudgetConfig);
+ }
+
+ private LogStorageBudget newBudget(LogStorageBudgetView logStorageBudgetConfig) {
+ LogStorageBudgetFactory factory = budgetFactories.get(logStorageBudgetConfig.name());
+
+ if (factory == null) {
+ throw new IgniteInternalException("Cannot find a log storage budget by name '" + logStorageBudgetConfig.name() + "'");
Review Comment:
Added a TODO to address this later
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/impl/VolatileLogStorageFactory.java:
##########
@@ -17,15 +17,64 @@
package org.apache.ignite.internal.raft.storage.impl;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.ServiceLoader;
+import java.util.Set;
+import org.apache.ignite.configuration.schemas.table.LogStorageBudgetView;
import org.apache.ignite.internal.raft.storage.LogStorageFactory;
+import org.apache.ignite.lang.IgniteInternalException;
+import org.apache.ignite.raft.jraft.core.LogStorageBudgetFactory;
+import org.apache.ignite.raft.jraft.core.LogStorageBudgetsModule;
import org.apache.ignite.raft.jraft.option.RaftOptions;
import org.apache.ignite.raft.jraft.storage.LogStorage;
import org.apache.ignite.raft.jraft.storage.impl.LocalLogStorage;
+import org.apache.ignite.raft.jraft.storage.impl.LogStorageBudget;
+import org.apache.ignite.raft.jraft.storage.impl.VolatileLogStorage;
/**
* Log storage factory based on {@link LocalLogStorage}.
*/
public class VolatileLogStorageFactory implements LogStorageFactory {
+ private final LogStorageBudgetView logStorageBudgetConfig;
+
+ private final Map<String, LogStorageBudgetFactory> budgetFactories;
+
+ /**
+ * Creates a new instance.
+ *
+ * @param logStorageBudgetConfig Budget config.
+ */
+ public VolatileLogStorageFactory(LogStorageBudgetView logStorageBudgetConfig) {
+ this.logStorageBudgetConfig = logStorageBudgetConfig;
+
+ Map<String, LogStorageBudgetFactory> factories = new HashMap<>();
+
+ ClassLoader serviceClassLoader = Thread.currentThread().getContextClassLoader();
+
+ for (LogStorageBudgetsModule module : ServiceLoader.load(LogStorageBudgetsModule.class, serviceClassLoader)) {
+ Map<String, LogStorageBudgetFactory> factoriesFromModule = module.budgetFactories();
+
+ checkForBudgetNameClashes(factories.keySet(), factoriesFromModule.keySet());
+
+ factories.putAll(factoriesFromModule);
+ }
+
+ budgetFactories = Map.copyOf(factories);
+ }
+
+ private void checkForBudgetNameClashes(Set<String> names1, Set<String> names2) {
+ Set<String> intersection = new HashSet<>(names1);
+ intersection.retainAll(names2);
+
+ if (!intersection.isEmpty()) {
+ throw new IgniteInternalException(
Review Comment:
Added a TODO to address this later
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
rpuch commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r947070511
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/impl/EntryCountBudget.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import java.util.List;
+import org.apache.ignite.raft.jraft.entity.LogEntry;
+
+/**
+ * {@link LogStorageBudget} that makes sure that no more entries than the provided limit is stored.
+ *
+ * <p>This is not thread safe as {@link LogStorageBudget} implementations do not need to be thread safe (because all budget methods
+ * are called under locks).
+ */
+public class EntryCountBudget implements LogStorageBudget {
+
+ private static final long NO_INDEX = -1;
+
+ private final long entriesCountLimit;
+
+ private long firstIndex = NO_INDEX;
+ private long lastIndex = NO_INDEX;
+
+ public EntryCountBudget(long entriesCountLimit) {
+ this.entriesCountLimit = entriesCountLimit;
+ }
+
+ @Override
+ public boolean hasRoomFor(LogEntry entry) {
Review Comment:
Later, we'll have another implementation that will account for total entries size, it will need entry.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r940912345
##########
.gitignore:
##########
@@ -4,3 +4,8 @@ target
work
.DS_Store
.flattened-pom.xml
+
+.classpath
Review Comment:
ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
rpuch commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r941003213
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/LogStorageBudgetsModule.java:
##########
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.raft.jraft.core;
+
+import java.util.Map;
+import java.util.function.Function;
+import org.apache.ignite.configuration.schemas.table.LogStorageBudgetView;
+import org.apache.ignite.raft.jraft.storage.impl.LogStorageBudget;
+
+/**
+ * Used to add {@link LogStorageBudget} factories using {@link java.util.ServiceLoader}.
+ */
+public interface LogStorageBudgetsModule {
+ /**
+ * Returns mapping from budget names to budget factories for the budgets supported by this module.
+ *
+ * @return Mapping from budget names to budget factories for the budgets supported by this module.
+ */
+ Map<String, Function<? super LogStorageBudgetView, LogStorageBudget>> budgetFactories();
Review Comment:
Created an interface, thanks for the suggestion. But it looks like view is enough here: we only need to read the config and don't need the flexibility given by the full `Config`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
rpuch commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r946899875
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/impl/VolatileLogStorage.java:
##########
@@ -0,0 +1,299 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import java.util.List;
+import java.util.NavigableMap;
+import java.util.SortedMap;
+import java.util.concurrent.ConcurrentSkipListMap;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.raft.jraft.entity.EnumOutter;
+import org.apache.ignite.raft.jraft.entity.LogEntry;
+import org.apache.ignite.raft.jraft.entity.LogId;
+import org.apache.ignite.raft.jraft.entity.codec.LogEntryDecoder;
+import org.apache.ignite.raft.jraft.entity.codec.LogEntryEncoder;
+import org.apache.ignite.raft.jraft.option.LogStorageOptions;
+import org.apache.ignite.raft.jraft.storage.LogStorage;
+import org.apache.ignite.raft.jraft.storage.VolatileStorage;
+import org.apache.ignite.raft.jraft.util.Describer;
+import org.apache.ignite.raft.jraft.util.Requires;
+
+/**
+ * Stores RAFT log in memory.
+ */
+public class VolatileLogStorage implements LogStorage, Describer, VolatileStorage {
+ private static final IgniteLogger LOG = Loggers.forClass(VolatileLogStorage.class);
+
+ private final LogStorageBudget budget;
+
+ private final ReadWriteLock readWriteLock = new ReentrantReadWriteLock();
+ private final Lock readLock = this.readWriteLock.readLock();
+ private final Lock writeLock = this.readWriteLock.writeLock();
+
+ private final NavigableMap<Long, LogEntry> log = new ConcurrentSkipListMap<>();
+
+ private LogEntryEncoder logEntryEncoder;
+ private LogEntryDecoder logEntryDecoder;
+
+ private volatile long firstLogIndex = 1;
+ private volatile long lastLogIndex = 0;
+
+ private volatile boolean initialized = false;
+
+ public VolatileLogStorage(LogStorageBudget budget) {
+ super();
+
+ this.budget = budget;
+ }
+
+ @Override
+ public boolean init(final LogStorageOptions opts) {
+ Requires.requireNonNull(opts.getConfigurationManager(), "Null conf manager");
+ Requires.requireNonNull(opts.getLogEntryCodecFactory(), "Null log entry codec factory");
+
+ this.writeLock.lock();
+
+ try {
+ if (initialized) {
+ LOG.warn("VolatileLogStorage init() was already called.");
+ return true;
+ }
+ this.initialized = true;
+ this.logEntryDecoder = opts.getLogEntryCodecFactory().decoder();
+ this.logEntryEncoder = opts.getLogEntryCodecFactory().encoder();
+ Requires.requireNonNull(this.logEntryDecoder, "Null log entry decoder");
+ Requires.requireNonNull(this.logEntryEncoder, "Null log entry encoder");
+
+ return true;
+ } finally {
+ this.writeLock.unlock();
+ }
+ }
+
+ @Override
+ public void shutdown() {
+ this.writeLock.lock();
+
+ try {
+ this.initialized = false;
+ this.log.clear();
+ } finally {
+ this.writeLock.unlock();
+ }
+ }
+
+ @Override
+ public long getFirstLogIndex() {
+ this.readLock.lock();
+
+ try {
+ return this.firstLogIndex;
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public long getLastLogIndex() {
+ this.readLock.lock();
+
+ try {
+ return this.lastLogIndex;
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public LogEntry getEntry(final long index) {
+ this.readLock.lock();
+
+ try {
+ if (index < getFirstLogIndex()) {
+ return null;
+ }
+
+ return log.get(index);
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public long getTerm(final long index) {
+ final LogEntry entry = getEntry(index);
+ if (entry != null) {
+ return entry.getId().getTerm();
+ }
+ return 0;
+ }
+
+ @Override
+ public boolean appendEntry(final LogEntry entry) {
+ this.readLock.lock();
+
+ try {
+ if (!initialized) {
+ LOG.warn("DB not initialized or destroyed.");
+ return false;
+ }
+
+ this.log.put(entry.getId().getIndex(), entry);
+
+ lastLogIndex = log.lastKey();
+ firstLogIndex = log.firstKey();
+
+ budget.onAppended(entry);
+
+ return true;
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public int appendEntries(final List<LogEntry> entries) {
+ if (entries == null || entries.isEmpty()) {
+ return 0;
+ }
+
+ final int entriesCount = entries.size();
+
+ this.readLock.lock();
+
+ try {
+ if (!initialized) {
+ LOG.warn("DB not initialized or destroyed.");
+ return 0;
+ }
+
+ int appended = 0;
+ for (LogEntry logEntry : entries) {
+
+ log.put(logEntry.getId().getIndex(), logEntry);
+
+ appended++;
+ }
+
+ lastLogIndex = log.lastKey();
+ firstLogIndex = log.firstKey();
+
+ budget.onAppended(entries.subList(0, appended));
+
+ if (appended < entriesCount) {
Review Comment:
Thanks for noting this! Removed this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
rpuch commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r940558869
##########
modules/api/src/main/java/org/apache/ignite/configuration/schemas/table/RaftConfigurationSchema.java:
##########
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.configuration.schemas.table;
+
+import org.apache.ignite.configuration.annotation.Config;
+import org.apache.ignite.configuration.annotation.Value;
+import org.apache.ignite.configuration.validation.Immutable;
+
+/**
+ * Configuration for Raft group corresponding to tables.
+ */
+@Config
+public class RaftConfigurationSchema {
+ /** Class of log storage budget. */
+ @Value(hasDefault = true)
+ @Immutable
+ public String logStorageBudgetClass = "org.apache.ignite.raft.jraft.storage.impl.UnlimitedBudget";
Review Comment:
Changed from class name to logical name (using ServiceLoader).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] alievmirza commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
alievmirza commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r946977713
##########
modules/api/src/main/java/org/apache/ignite/configuration/schemas/table/UnlimitedBudgetConfigurationSchema.java:
##########
@@ -0,0 +1,29 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.configuration.schemas.table;
+
+import org.apache.ignite.configuration.annotation.PolymorphicConfigInstance;
+
+/**
+ * Configuration for 'unlimited' log storage budget.
Review Comment:
Let's add here link to the corresponding interface instead of 'unlimited' phrase.
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/impl/EntryCountBudget.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import java.util.List;
+import org.apache.ignite.raft.jraft.entity.LogEntry;
+
+/**
+ * {@link LogStorageBudget} that makes sure that no more entries than the provided limit is stored.
+ *
+ * <p>This is not thread safe as {@link LogStorageBudget} implementations do not need to be thread safe (because all budget methods
+ * are called under locks).
+ */
+public class EntryCountBudget implements LogStorageBudget {
+
+ private static final long NO_INDEX = -1;
+
+ private final long entriesCountLimit;
+
+ private long firstIndex = NO_INDEX;
+ private long lastIndex = NO_INDEX;
+
+ public EntryCountBudget(long entriesCountLimit) {
+ this.entriesCountLimit = entriesCountLimit;
+ }
+
+ @Override
+ public boolean hasRoomFor(LogEntry entry) {
Review Comment:
Seems like we don't need entry here, as long as we don't need it in all implementations of `org.apache.ignite.raft.jraft.storage.impl.LogStorageBudget#hasRoomFor`, maybe we can simplify it?
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/impl/VolatileLogStorageFactory.java:
##########
@@ -17,15 +17,64 @@
package org.apache.ignite.internal.raft.storage.impl;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.ServiceLoader;
+import java.util.Set;
+import org.apache.ignite.configuration.schemas.table.LogStorageBudgetView;
import org.apache.ignite.internal.raft.storage.LogStorageFactory;
+import org.apache.ignite.lang.IgniteInternalException;
+import org.apache.ignite.raft.jraft.core.LogStorageBudgetFactory;
+import org.apache.ignite.raft.jraft.core.LogStorageBudgetsModule;
import org.apache.ignite.raft.jraft.option.RaftOptions;
import org.apache.ignite.raft.jraft.storage.LogStorage;
import org.apache.ignite.raft.jraft.storage.impl.LocalLogStorage;
+import org.apache.ignite.raft.jraft.storage.impl.LogStorageBudget;
+import org.apache.ignite.raft.jraft.storage.impl.VolatileLogStorage;
/**
* Log storage factory based on {@link LocalLogStorage}.
Review Comment:
Probably, here should be `based on VolatileLogStorage`
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/impl/VolatileLogStorageFactory.java:
##########
@@ -17,15 +17,64 @@
package org.apache.ignite.internal.raft.storage.impl;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.ServiceLoader;
+import java.util.Set;
+import org.apache.ignite.configuration.schemas.table.LogStorageBudgetView;
import org.apache.ignite.internal.raft.storage.LogStorageFactory;
+import org.apache.ignite.lang.IgniteInternalException;
+import org.apache.ignite.raft.jraft.core.LogStorageBudgetFactory;
+import org.apache.ignite.raft.jraft.core.LogStorageBudgetsModule;
import org.apache.ignite.raft.jraft.option.RaftOptions;
import org.apache.ignite.raft.jraft.storage.LogStorage;
import org.apache.ignite.raft.jraft.storage.impl.LocalLogStorage;
+import org.apache.ignite.raft.jraft.storage.impl.LogStorageBudget;
+import org.apache.ignite.raft.jraft.storage.impl.VolatileLogStorage;
/**
* Log storage factory based on {@link LocalLogStorage}.
*/
public class VolatileLogStorageFactory implements LogStorageFactory {
+ private final LogStorageBudgetView logStorageBudgetConfig;
+
+ private final Map<String, LogStorageBudgetFactory> budgetFactories;
+
+ /**
+ * Creates a new instance.
+ *
+ * @param logStorageBudgetConfig Budget config.
+ */
+ public VolatileLogStorageFactory(LogStorageBudgetView logStorageBudgetConfig) {
+ this.logStorageBudgetConfig = logStorageBudgetConfig;
+
+ Map<String, LogStorageBudgetFactory> factories = new HashMap<>();
+
+ ClassLoader serviceClassLoader = Thread.currentThread().getContextClassLoader();
+
+ for (LogStorageBudgetsModule module : ServiceLoader.load(LogStorageBudgetsModule.class, serviceClassLoader)) {
+ Map<String, LogStorageBudgetFactory> factoriesFromModule = module.budgetFactories();
+
+ checkForBudgetNameClashes(factories.keySet(), factoriesFromModule.keySet());
+
+ factories.putAll(factoriesFromModule);
+ }
+
+ budgetFactories = Map.copyOf(factories);
+ }
+
+ private void checkForBudgetNameClashes(Set<String> names1, Set<String> names2) {
+ Set<String> intersection = new HashSet<>(names1);
+ intersection.retainAll(names2);
+
+ if (!intersection.isEmpty()) {
+ throw new IgniteInternalException(
Review Comment:
Is there any possibility to not use deprecated constructor for IgniteInternalException?
Maybe we should introduce some error codes for that scenario
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/impl/EntryCountBudget.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import java.util.List;
+import org.apache.ignite.raft.jraft.entity.LogEntry;
+
+/**
+ * {@link LogStorageBudget} that makes sure that no more entries than the provided limit is stored.
+ *
+ * <p>This is not thread safe as {@link LogStorageBudget} implementations do not need to be thread safe (because all budget methods
+ * are called under locks).
+ */
+public class EntryCountBudget implements LogStorageBudget {
+
+ private static final long NO_INDEX = -1;
+
+ private final long entriesCountLimit;
+
+ private long firstIndex = NO_INDEX;
+ private long lastIndex = NO_INDEX;
+
+ public EntryCountBudget(long entriesCountLimit) {
+ this.entriesCountLimit = entriesCountLimit;
+ }
+
+ @Override
+ public boolean hasRoomFor(LogEntry entry) {
+ return storedEntries() < entriesCountLimit;
+ }
+
+ private long storedEntries() {
+ if (firstIndex == NO_INDEX && lastIndex == NO_INDEX) {
+ return 0;
+ } else if (firstIndex != NO_INDEX && lastIndex != NO_INDEX) {
+ return lastIndex - firstIndex + 1;
+ } else {
+ throw new IllegalStateException("Only one of firstIndex and lastIndex is initialized: " + firstIndex + " and " + lastIndex);
+ }
+ }
+
+ @Override
+ public void onAppended(LogEntry entry) {
Review Comment:
Seems, that we don't need the whole entry here, we can use only corresponding index. Let's simplify it and use `long` here and in `EntryCountBudget#onAppended(java.util.List<org.apache.ignite.raft.jraft.entity.LogEntry>)`
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/server/RaftGroupOptions.java:
##########
@@ -64,6 +64,16 @@ public static RaftGroupOptions forVolatileStores() {
return new RaftGroupOptions(true);
}
+ /**
+ * Creates options derived from table configuration.
+ *
+ * @param isVolatile Whether the table is configured as volatile (in-memory) or not.
+ * @return Options derived from table configuration.
+ */
+ public static RaftGroupOptions forTable(boolean isVolatile) {
Review Comment:
Why do we need this? I don't see any usages
##########
modules/api/src/main/java/org/apache/ignite/configuration/schemas/table/EntryCountBudgetConfigurationSchema.java:
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.configuration.schemas.table;
+
+import org.apache.ignite.configuration.annotation.PolymorphicConfigInstance;
+import org.apache.ignite.configuration.annotation.Value;
+
+/**
+ * Configuration for 'entry-count' log storage budget.
Review Comment:
Let's add here link to the corresponding interface instead of 'entry-count' phrase. It is quite unclear from the context what we are talking about
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/impl/VolatileLogStorageFactory.java:
##########
@@ -34,7 +83,21 @@ public void start() {
/** {@inheritDoc} */
@Override
public LogStorage createLogStorage(String uri, RaftOptions raftOptions) {
- return new LocalLogStorage(raftOptions);
+ return new VolatileLogStorage(createLogStorageBudget());
+ }
+
+ private LogStorageBudget createLogStorageBudget() {
+ return newBudget(logStorageBudgetConfig);
+ }
+
+ private LogStorageBudget newBudget(LogStorageBudgetView logStorageBudgetConfig) {
+ LogStorageBudgetFactory factory = budgetFactories.get(logStorageBudgetConfig.name());
+
+ if (factory == null) {
+ throw new IgniteInternalException("Cannot find a log storage budget by name '" + logStorageBudgetConfig.name() + "'");
Review Comment:
Is there any possibility to not use deprecated constructor for` IgniteInternalException`?
Maybe we should introduce some error codes for that scenario
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/impl/VolatileLogStorage.java:
##########
@@ -0,0 +1,289 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import java.util.List;
+import java.util.NavigableMap;
+import java.util.SortedMap;
+import java.util.concurrent.ConcurrentSkipListMap;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.raft.jraft.entity.EnumOutter;
+import org.apache.ignite.raft.jraft.entity.LogEntry;
+import org.apache.ignite.raft.jraft.entity.LogId;
+import org.apache.ignite.raft.jraft.entity.codec.LogEntryDecoder;
+import org.apache.ignite.raft.jraft.entity.codec.LogEntryEncoder;
+import org.apache.ignite.raft.jraft.option.LogStorageOptions;
+import org.apache.ignite.raft.jraft.storage.LogStorage;
+import org.apache.ignite.raft.jraft.storage.VolatileStorage;
+import org.apache.ignite.raft.jraft.util.Describer;
+import org.apache.ignite.raft.jraft.util.Requires;
+
+/**
+ * Stores RAFT log in memory.
+ */
+public class VolatileLogStorage implements LogStorage, Describer, VolatileStorage {
+ private static final IgniteLogger LOG = Loggers.forClass(VolatileLogStorage.class);
+
+ private final LogStorageBudget budget;
+
+ private final ReadWriteLock readWriteLock = new ReentrantReadWriteLock();
+ private final Lock readLock = this.readWriteLock.readLock();
+ private final Lock writeLock = this.readWriteLock.writeLock();
+
+ private final NavigableMap<Long, LogEntry> log = new ConcurrentSkipListMap<>();
+
+ private LogEntryEncoder logEntryEncoder;
+ private LogEntryDecoder logEntryDecoder;
+
+ private volatile long firstLogIndex = 1;
+ private volatile long lastLogIndex = 0;
+
+ private volatile boolean initialized = false;
+
+ public VolatileLogStorage(LogStorageBudget budget) {
+ super();
+
+ this.budget = budget;
+ }
+
+ @Override
+ public boolean init(final LogStorageOptions opts) {
+ Requires.requireNonNull(opts.getConfigurationManager(), "Null conf manager");
+ Requires.requireNonNull(opts.getLogEntryCodecFactory(), "Null log entry codec factory");
+
+ this.writeLock.lock();
+
+ try {
+ if (initialized) {
+ LOG.warn("VolatileLogStorage init() was already called.");
+ return true;
+ }
+ this.initialized = true;
+ this.logEntryDecoder = opts.getLogEntryCodecFactory().decoder();
+ this.logEntryEncoder = opts.getLogEntryCodecFactory().encoder();
+ Requires.requireNonNull(this.logEntryDecoder, "Null log entry decoder");
+ Requires.requireNonNull(this.logEntryEncoder, "Null log entry encoder");
+
+ return true;
+ } finally {
+ this.writeLock.unlock();
+ }
+ }
+
+ @Override
+ public void shutdown() {
+ this.writeLock.lock();
+
+ try {
+ this.initialized = false;
+ this.log.clear();
+ } finally {
+ this.writeLock.unlock();
+ }
+ }
+
+ @Override
+ public long getFirstLogIndex() {
+ this.readLock.lock();
+
+ try {
+ return this.firstLogIndex;
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public long getLastLogIndex() {
+ this.readLock.lock();
+
+ try {
+ return this.lastLogIndex;
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public LogEntry getEntry(final long index) {
+ this.readLock.lock();
+
+ try {
+ if (index < getFirstLogIndex()) {
+ return null;
+ }
+
+ return log.get(index);
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public long getTerm(final long index) {
+ final LogEntry entry = getEntry(index);
+ if (entry != null) {
+ return entry.getId().getTerm();
+ }
+ return 0;
+ }
+
+ @Override
+ public boolean appendEntry(final LogEntry entry) {
+ this.readLock.lock();
+
+ try {
+ if (!initialized) {
+ LOG.warn("DB not initialized or destroyed.");
+ return false;
+ }
+
+ this.log.put(entry.getId().getIndex(), entry);
+
+ lastLogIndex = log.lastKey();
+ firstLogIndex = log.firstKey();
+
+ budget.onAppended(entry);
+
+ return true;
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public int appendEntries(final List<LogEntry> entries) {
+ if (entries == null || entries.isEmpty()) {
+ return 0;
+ }
+
+ final int entriesCount = entries.size();
+
+ this.readLock.lock();
+
+ try {
+ if (!initialized) {
+ LOG.warn("DB not initialized or destroyed.");
+ return 0;
+ }
+
+ for (LogEntry logEntry : entries) {
+ log.put(logEntry.getId().getIndex(), logEntry);
+ }
+
+ lastLogIndex = log.lastKey();
+ firstLogIndex = log.firstKey();
+
+ budget.onAppended(entries);
Review Comment:
If this will be added later, let's add at least todo with the corresponding ticket
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/impl/VolatileLogStorage.java:
##########
@@ -0,0 +1,289 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ignite.raft.jraft.storage.impl;
+
+import java.util.List;
+import java.util.NavigableMap;
+import java.util.SortedMap;
+import java.util.concurrent.ConcurrentSkipListMap;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.raft.jraft.entity.EnumOutter;
+import org.apache.ignite.raft.jraft.entity.LogEntry;
+import org.apache.ignite.raft.jraft.entity.LogId;
+import org.apache.ignite.raft.jraft.entity.codec.LogEntryDecoder;
+import org.apache.ignite.raft.jraft.entity.codec.LogEntryEncoder;
+import org.apache.ignite.raft.jraft.option.LogStorageOptions;
+import org.apache.ignite.raft.jraft.storage.LogStorage;
+import org.apache.ignite.raft.jraft.storage.VolatileStorage;
+import org.apache.ignite.raft.jraft.util.Describer;
+import org.apache.ignite.raft.jraft.util.Requires;
+
+/**
+ * Stores RAFT log in memory.
+ */
+public class VolatileLogStorage implements LogStorage, Describer, VolatileStorage {
+ private static final IgniteLogger LOG = Loggers.forClass(VolatileLogStorage.class);
+
+ private final LogStorageBudget budget;
+
+ private final ReadWriteLock readWriteLock = new ReentrantReadWriteLock();
+ private final Lock readLock = this.readWriteLock.readLock();
+ private final Lock writeLock = this.readWriteLock.writeLock();
+
+ private final NavigableMap<Long, LogEntry> log = new ConcurrentSkipListMap<>();
+
+ private LogEntryEncoder logEntryEncoder;
+ private LogEntryDecoder logEntryDecoder;
+
+ private volatile long firstLogIndex = 1;
+ private volatile long lastLogIndex = 0;
+
+ private volatile boolean initialized = false;
+
+ public VolatileLogStorage(LogStorageBudget budget) {
+ super();
+
+ this.budget = budget;
+ }
+
+ @Override
+ public boolean init(final LogStorageOptions opts) {
+ Requires.requireNonNull(opts.getConfigurationManager(), "Null conf manager");
+ Requires.requireNonNull(opts.getLogEntryCodecFactory(), "Null log entry codec factory");
+
+ this.writeLock.lock();
+
+ try {
+ if (initialized) {
+ LOG.warn("VolatileLogStorage init() was already called.");
+ return true;
+ }
+ this.initialized = true;
+ this.logEntryDecoder = opts.getLogEntryCodecFactory().decoder();
+ this.logEntryEncoder = opts.getLogEntryCodecFactory().encoder();
+ Requires.requireNonNull(this.logEntryDecoder, "Null log entry decoder");
+ Requires.requireNonNull(this.logEntryEncoder, "Null log entry encoder");
+
+ return true;
+ } finally {
+ this.writeLock.unlock();
+ }
+ }
+
+ @Override
+ public void shutdown() {
+ this.writeLock.lock();
+
+ try {
+ this.initialized = false;
+ this.log.clear();
+ } finally {
+ this.writeLock.unlock();
+ }
+ }
+
+ @Override
+ public long getFirstLogIndex() {
+ this.readLock.lock();
+
+ try {
+ return this.firstLogIndex;
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public long getLastLogIndex() {
+ this.readLock.lock();
+
+ try {
+ return this.lastLogIndex;
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public LogEntry getEntry(final long index) {
+ this.readLock.lock();
+
+ try {
+ if (index < getFirstLogIndex()) {
+ return null;
+ }
+
+ return log.get(index);
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public long getTerm(final long index) {
+ final LogEntry entry = getEntry(index);
+ if (entry != null) {
+ return entry.getId().getTerm();
+ }
+ return 0;
+ }
+
+ @Override
+ public boolean appendEntry(final LogEntry entry) {
+ this.readLock.lock();
+
+ try {
+ if (!initialized) {
+ LOG.warn("DB not initialized or destroyed.");
+ return false;
+ }
+
+ this.log.put(entry.getId().getIndex(), entry);
+
+ lastLogIndex = log.lastKey();
+ firstLogIndex = log.firstKey();
+
+ budget.onAppended(entry);
+
+ return true;
+ } finally {
+ this.readLock.unlock();
+ }
+ }
+
+ @Override
+ public int appendEntries(final List<LogEntry> entries) {
+ if (entries == null || entries.isEmpty()) {
+ return 0;
+ }
+
+ final int entriesCount = entries.size();
+
+ this.readLock.lock();
+
+ try {
+ if (!initialized) {
+ LOG.warn("DB not initialized or destroyed.");
+ return 0;
+ }
+
+ for (LogEntry logEntry : entries) {
+ log.put(logEntry.getId().getIndex(), logEntry);
+ }
+
+ lastLogIndex = log.lastKey();
+ firstLogIndex = log.firstKey();
+
+ budget.onAppended(entries);
Review Comment:
I don't see any outcomes when there is no room for entries, why do we even need this budget logic then?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
rpuch commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r947075108
##########
modules/api/src/main/java/org/apache/ignite/configuration/schemas/table/UnlimitedBudgetConfigurationSchema.java:
##########
@@ -0,0 +1,29 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.configuration.schemas.table;
+
+import org.apache.ignite.configuration.annotation.PolymorphicConfigInstance;
+
+/**
+ * Configuration for 'unlimited' log storage budget.
Review Comment:
The corresponding interface (and its implementations) are not available in this module, so it's impossible to add a link to them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #939: IGNITE-17334 Basic volatile RAFT log storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #939:
URL: https://github.com/apache/ignite-3/pull/939#discussion_r945502659
##########
modules/api/src/main/java/org/apache/ignite/configuration/schemas/table/EntryCountBudgetConfigurationSchema.java:
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.configuration.schemas.table;
+
+import org.apache.ignite.configuration.annotation.PolymorphicConfigInstance;
+import org.apache.ignite.configuration.annotation.Value;
+
+/**
+ * Configuration for 'entry-count' log storage budget.
+ */
+@PolymorphicConfigInstance(EntryCountBudgetConfigurationSchema.NAME)
+public class EntryCountBudgetConfigurationSchema extends LogStorageBudgetConfigurationSchema {
+ /** The budget name. */
+ public static final String NAME = "entry-count";
+
+ /** Returns maximum number of entries allowed by the budget. */
Review Comment:
I think there is a typo here.
##########
modules/api/src/main/java/org/apache/ignite/configuration/schemas/table/VolatileRaftConfigurationSchema.java:
##########
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.configuration.schemas.table;
+
+import org.apache.ignite.configuration.annotation.Config;
+import org.apache.ignite.configuration.annotation.ConfigValue;
+
+/**
+ * Configuration for Raft group corresponding to tables.
Review Comment:
Can you specify what is for volatile?
##########
modules/api/src/main/java/org/apache/ignite/configuration/schemas/table/LogStorageBudgetConfigurationSchema.java:
##########
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.configuration.schemas.table;
+
+import org.apache.ignite.configuration.annotation.PolymorphicConfig;
+import org.apache.ignite.configuration.annotation.PolymorphicId;
+
+/**
+ * Configuration schema for RAFT log storage budget.
+ */
+@PolymorphicConfig
+public class LogStorageBudgetConfigurationSchema {
+ /** Name of data storage. */
Review Comment:
I think there is a typo here.
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/impl/VolatileLogStorageFactory.java:
##########
@@ -17,15 +17,63 @@
package org.apache.ignite.internal.raft.storage.impl;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.ServiceLoader;
+import java.util.Set;
+import org.apache.ignite.configuration.schemas.table.LogStorageBudgetView;
import org.apache.ignite.internal.raft.storage.LogStorageFactory;
+import org.apache.ignite.lang.IgniteInternalException;
+import org.apache.ignite.raft.jraft.core.LogStorageBudgetFactory;
+import org.apache.ignite.raft.jraft.core.LogStorageBudgetsModule;
import org.apache.ignite.raft.jraft.option.RaftOptions;
import org.apache.ignite.raft.jraft.storage.LogStorage;
import org.apache.ignite.raft.jraft.storage.impl.LocalLogStorage;
+import org.apache.ignite.raft.jraft.storage.impl.LogStorageBudget;
+import org.apache.ignite.raft.jraft.storage.impl.VolatileLogStorage;
/**
* Log storage factory based on {@link LocalLogStorage}.
*/
public class VolatileLogStorageFactory implements LogStorageFactory {
+ private final LogStorageBudgetView logStorageBudgetConfig;
+
+ private final Map<String, LogStorageBudgetFactory> budgetFactories;
+
+ /**
+ * Creates a new instance.
+ *
+ * @param logStorageBudgetConfig Budget config.
+ */
+ public VolatileLogStorageFactory(LogStorageBudgetView logStorageBudgetConfig) {
+ this.logStorageBudgetConfig = logStorageBudgetConfig;
+
+ Map<String, LogStorageBudgetFactory> factories = new HashMap<>();
+
+ ClassLoader serviceClassLoader = Thread.currentThread().getContextClassLoader();
+ for (LogStorageBudgetsModule module : ServiceLoader.load(LogStorageBudgetsModule.class, serviceClassLoader)) {
Review Comment:
```suggestion
ClassLoader serviceClassLoader = Thread.currentThread().getContextClassLoader();
for (LogStorageBudgetsModule module : ServiceLoader.load(LogStorageBudgetsModule.class, serviceClassLoader)) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org