ArrayIndexOutOfBoundsException w cache

0

Co kilkadziesiąt sekund 1 wątek woła cachedSimpleService.update() i równocześnie cały czas miliardy klientów uderza przez http do controlera, który woła cachedSimpleService.getXmlData(${page}). Co kilka dni aplikacja się wykrzacza, wchodzi w zły stan i trzeba ją zrestartować. Wylatuje ArrayIndexOutOfBoundsException na każdy cachedSimpleService.getXmlData(${page}).

Weak, "eventual consistancy" jest wystarczające, klienty nie muszą widzieć tych samych danych od razu ani aktualnych, ważne by w końcu za którymś zapytaniem się odświeżyły dlatego nic tu nie gmerałem z blokami synchronized itp.

Widzicie jakiś błąd w tym kodzie?

class CachedSimpleService implements MyService {

    private final SimpleService myService;
    private Map<Integer, String> xmlDataPaged;

    public CachedSimpleService(MyService myService) {
        this.myService = myService;
        this.dataPaged = new ConcurrentHashMap<>();
    }

    @Override
    public String getXmlData(int page) {
        String xml = xmlDataPaged.get(page);
        if (xml == null) {
            xml = myService.getXmlData(page);
            xmlDataPaged.put(page, xml);
        }
        return xml;
    }

    @Override
    public void update() {
        this.myService.update();
        invalidateCache();
    }

    private void invalidateCache() {
        this.xmlDataPaged = new ConcurrentHashMap<>();
    }
}

class SimpleService implements MyService {

    private final DataHolder dataHolder;
    private final PageableMapper pageableMapper;
    private final XmlParser xmlParser;

    @Override
    public String getXmlData(int page) {
        var data = dataHolder.getData();
        var singlePageFromData = pageableMapper.from(data, page);
        return xmlParser.parse(singlePageFromData);              // ===> tu leci java.lang.ArrayIndexOutOfBoundsException: Index -1 out of bounds for length 47
    }

    @Override
    public void update() {
        this.dataHolder.update();
    }
}
class DataHolder {

    private final DataRepo dataRepo;

    private Data data;

    public Data getData() {
        if (data == null) {
            throw new DataNotInitializedException("Data not prepared yet. Please try again later");
        }
        return data;
    }

    public void update() {
        this.data = getData();
    }

    private Data getData() {
      // ...
    }
}


zrobiłbym tak, ale nie wiem czy to jest przyczyną tych błędów:


class CachedSimpleService implements MyService {

    private final SimpleService myService;
    private volatile Map<Integer, String> xmlDataPaged;

    public CachedSimpleService(MyService myService) {
        this.myService = myService;
        this.dataPaged = new ConcurrentHashMap<>();
    }

    @Override
    public String getXmlData(int page) {
        String xml = xmlDataPaged.get(page);
        if (xml == null) {z
            synchronized(xmlDataPaged) {                // ===> changed
                if(xml == null {                        // ===> changed
                    xml = myService.getXmlData(page);
                    xmlDataPaged.put(page, xml);
                }
            }
        }
        return xml;
    }

    @Override
    public void update() {
        this.myService.update();
        invalidateCache();
    }

    private void invalidateCache() {
        this.xmlDataPaged = new ConcurrentHashMap<>();
    }
}

class DataHolder {

    private final DataRepo dataRepo;

    private volatile Data data;

    public synchronized Data getData() { // ===> changed
        if (data == null) {
            throw new DataNotInitializedException("Data not prepared yet. Please try again later");
        }
        return data;
    }

    public synchronized void update() { // ===> changed
        this.data = getData();
    }

    private Data getData() {
      // ...
    }
}

1
  1. return xmlParser.parse(singlePageFromData); jakie będzie zachowanie tego parsera, gdy raz na milion wywołań dostanie niepoprawny dokument XML?
    ) Przy invalidateCache mam wątpliwości, czy zmiana będzie atomowa, czy atomowa że wybuchnie na grubo ;-)
  private void invalidateCache() {
        this.xmlDataPaged = new ConcurrentHashMap<>();
    }

Dlaczego by tego ConcurrentHashMap nie opakować w AtomicReference ? Wówczas podmiana byłaby atomowa, a tak to kto to może wiedzieć.

0
    @Override
    public String getXmlData(int page) {
        String xml = xmlDataPaged.get(page);
        if (xml == null) {z
            synchronized(xmlDataPaged) {
                if(xml == null {
                    xml = myService.getXmlData(page);
                    xmlDataPaged.put(page, xml);
                }
            }
        }
        return xml;
    }

Użyj po prostu computeIfAbsent

0
crejk napisał(a):
    @Override
    public String getXmlData(int page) {
        String xml = xmlDataPaged.get(page);
        if (xml == null) {z
            synchronized(xmlDataPaged) {
                if(xml == null {
                    xml = myService.getXmlData(page);
                    xmlDataPaged.put(page, xml);
                }
            }
        }
        return xml;
    }

Użyj po prostu computeIfAbsent

haha, też się na to naciąłem. Przy computeIfAbsent mapa prędzej czy później będzie strzelać ConcurrentModificationException.

https://stackoverflow.com/questions/54824656/since-java-9-hashmap-computeifabsent-throws-concurrentmodificationexception-on

0
yarel napisał(a):
  1. return xmlParser.parse(singlePageFromData); jakie będzie zachowanie tego parsera, gdy raz na milion wywołań dostanie niepoprawny dokument XML?

właśnie, uzywam parsera z pakietu jakarta.xml może inicjować go za każdym użyciem:

        try {
            return init(MyDtoXml.class).parse(dto);
        } catch (JAXBException e) {
            throw new DataParsingException(e.getMessage());
        }

    init(Class<?> clazz) throws JAXBException {
        var marshaller = JAXBContext.newInstance(clazz).createMarshaller();
        marshaller.setProperty(Marshaller.JAXB_FORMATTED_OUTPUT, true);
        return new JakartaXmlParser(marshaller)
    }

bo miałem go jako Beana springowego czyli singleton.

0

JAXB wieki temu używałem, więc nie powiem Ci jak go teraz używać. Sprawdź sobie dokumentację czy ten parser jest thread safe, czy przypadkiem marshaller/unmarshaller nie utrzymują wewnętrznego stanu (to mogłoby być problemem w połączeniu z singletonem).

0

https://javaee.github.io/jaxb-v2/doc/user-guide/ch03.html#other-miscellaneous-topics-performance-and-thread-safety

the Marshaller, Unmarshaller, and Validator classes are not thread safe. (...) each time you need to unmarshal/marshal/validate a document. Just create a new Unmarshaller/Marshaller/Validator

3
  1. computeIfAbsent oczywiście można użyć i polecam (i nic nie zepsuje - wątki/problemy z so dotyczą czegoś zupełnie innego, nie mającego nic wspólnego z twoim kodem).
  2. Problem jest tu:
private void invalidateCache() {
        this.xmlDataPaged = new ConcurrentHashMap<>();
    }

A) pole powinno być final
B) a w tej metodzie powinieneś robić
this.xmlDataPaged.clear()

0

W takich wypadkach jak ktoś nie umie w wątki a się pcha, to od razu blokuję PR i karzę użyć jednej z uznanych bibliotek:

Ludzie którzy nawet się znają na wielowątkowości mogą takie bugi do kodu wprowadzić że 100 thread dumpów nie pomoże rozwiązać problemu.

0

Nie możesz stacktrace wyświetlić? jakie były informacje w ramce przed wyjątkiem?
Takie gdb pozwala dowolną ilość ramek przeskoczyć do góry i na dół do wyjątku, możesz do dowolnej funkcji jaka była wcześniej wywołana i dalej leży na stosie.

Chodź oczywiście rozumiem, że to losowo wywala błąd, to nie jest tak łatwo się na niego przyczaić.
Dumpy też są fajne, ale jak jesteś dynamicznie podłączony debuggerem to możesz sobie swobodnie się przemieszczać po programie jak umarł.

0

https://javaee.github.io/jaxb-v2/doc/user-guide/ch03.html#other-miscellaneous-topics-performance-and-thread-safety

the Marshaller, Unmarshaller, and Validator classes are not thread safe. (...) each time you need to unmarshal/marshal/validate a document. Just create a new Unmarshaller/Marshaller/Validator

to chyba rozwiązało problem.

1 użytkowników online, w tym zalogowanych: 0, gości: 1