Skip to content
This repository has been archived by the owner on Mar 3, 2024. It is now read-only.

Ершов Вадим, Магистратура ИТМО "Распределенные веб-сервисы", hw 5 #291

Closed
wants to merge 15 commits into from

Conversation

vadim01er
Copy link
Contributor

No description provided.

Copy link
Member

@incubos incubos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут как таковой попытки неблокирующей реализации и не было, верно? Появился только ReadWriteLock. Могу оценить в 1 балл за начала подхода к попытке.

public class ThreadSafeDaoImpl implements Dao<MemorySegment, Entry<MemorySegment>> {

private final Comparator<MemorySegment> comparator = ThreadSafeDaoImpl::compare;
private NavigableMap<MemorySegment, Entry<MemorySegment>> storage = new ConcurrentSkipListMap<>(comparator);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Поле storage читается/пишется из разных потоков. Чтения не защищены.

public void flush() {
w.lock();
try {
DiskStorage.save(path, storage.values());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Почему мы не очищаем storage (MemTable) после flush и не переоткрываем флашнутую таблицу?

}
iterators.add(firstIterator);

return new MergeIterator<>(iterators, Comparator.comparing(Entry::key, ThreadSafeDaoImpl::compare));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Мы обращаемся к полю ThreadSafeDaoImpl? Текут абстракции...


// private final ExecutorService executorService = Executors.newSingleThreadExecutor();

private long currentBytes = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Поле модифицируется из разных потоков под гонкой.

if (currentBytes > flushThresholdBytes) {
flush();
}
currentBytes += entry.key().byteSize();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Операции инкремента currentBytes в таком виде не атомарны.

DiskStorage.save(path, storage.values());
currentBytes = 0;
} catch (IOException e) {
throw new RuntimeException(e);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
throw new RuntimeException(e);
throw new UncheckedIOException(e);

}

@Override
public void flush() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Операция блокирующая и не соответствует спецификации, верно?

}

@Override
public void compact() throws IOException {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Операция блокирующая и не соответствует спецификации, верно?

try {
Files.createDirectories(compactPath);
DiskStorage.save(compactPath, this::all);
DiskStorage.deleteFiles(path);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Почему мы не переключаемся на результат compaction, а продолжаем использовать удалённые файлы?

private final Path compactPath;
private final long flushThresholdBytes;

private final ReentrantReadWriteLock rwl = new ReentrantReadWriteLock();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private final ReentrantReadWriteLock rwl = new ReentrantReadWriteLock();
private final ReadWriteLock rwl = new ReentrantReadWriteLock();

@incubos incubos closed this Dec 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants